Re: [Xen-devel] [PATCH] vt-d: use two 32-bit writes to update DMAR fault address registers

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Roger Pau Monné [mailto:roger.pau@xxxxxxxxxx]
> Sent: Monday, September 18, 2017 5:10 PM
> 
> On Mon, Sep 18, 2017 at 05:05:18PM +0800, Haozhong Zhang wrote:
> > On 09/18/17 02:30 -0600, Jan Beulich wrote:
> > > >>> On 18.09.17 at 10:18, <kevin.tian@xxxxxxxxx> wrote:
> > > >>  From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > > >> Sent: Monday, September 11, 2017 6:03 PM
> > > >>
> > > >> >>> On 11.09.17 at 08:00, <haozhong.zhang@xxxxxxxxx> wrote:
> > > >> > The 64-bit DMAR fault address is composed of two 32 bits registers
> > > >> > DMAR_FEADDR_REG and DMAR_FEUADDR_REG. According to VT-d
> spec:
> > > >> > "Software is expected to access 32-bit registers as aligned
> doublewords",
> > > >> > a hypervisor should use two 32-bit writes to DMAR_FEADDR_REG
> and
> > > >> > DMAR_FEUADDR_REG separately in order to update a 64-bit fault
> > > >> address,
> > > >> > rather than a 64-bit write to DMAR_FEADDR_REG.
> > > >> >
> > > >> > Though I haven't seen any errors caused by such one 64-bit write
> on
> > > >> > real machines, it's still better to follow the specification.
> > > >>
> > > >> Any sane chipset should split qword accesses into dword ones if
> > > >> they can't be handled at some layer. Also if you undo something
> > > >> explicitly done by an earlier commit, please quote that commit
> > > >> and say what was wrong. After all Kevin as the VT-d maintainer
> > > >> agreed with the change back then.
> > > >
> > > > I'm OK with this change.
> > >
> > > Hmm, would you mind explaining? You were also okay with the
> > > change in the opposite direction back then, and we've had no
> > > reports of problems.
> > >
> >
> > I haven't seen any issues of the current 64-bit write on recent Intel
> > Haswell, Broadwell and Skylake Xeon platforms, so I guess the hardware
> > can properly handle the 64-bits write to contiguous 32-bit registers.
> >
> > I actually encountered errors when running Xen on KVM/QEMU with
> QEMU
> > vIOMMU enabled, which (QEMU) disallows 64-bit writes to 32-bit
> > registers and aborts if such writes happen.
> >
> > If this patch is considered senseless (as it does not fix any errors
> > on real hardware), I'm fine to fix the above abort on QEMU side (i.e.,
> > let vIOMMU in QEMU follow the behavior of real hardware).
> 
> I think that either the spec is changed to mention that quad-word
> accesses are allowed, or this patch is applied.
> 
> There's nothing wrong with the QEMU implementation, it adheres to the
> spec. So unless the spec is changed, we might see issues with other
> emulated DMAR units.
> 

I checked with our hardware guy. It' recommended to strictly
follow what spec says. there is no hardware-level guarantee 
that a 64b write will touch both FEADDR and FEUADDR. So 
we should fix Xen side.

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel