[Qemu-devel] Re: [PATCH 3/7] AMD IOMMU emulation

[Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>]

On Sat, Aug 28, 2010 at 03:58:23PM +0000, Blue Swirl wrote:
> On Sat, Aug 28, 2010 at 2:54 PM, Eduard - Gabriel Munteanu
> <eduard.munteanu@linux360.ro> wrote:
> > This introduces emulation for the AMD IOMMU, described in "AMD I/O
> > Virtualization Technology (IOMMU) Specification".
> >
> > Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> > ---

[snip]

> > diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c
> > new file mode 100644
> > index 0000000..43e0426
> > --- /dev/null
> > +++ b/hw/amd_iommu.c

[snip]

> > +static void amd_iommu_update_mmio(AMDIOMMUState *st,
> > + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??target_phys_addr_t addr)
> > +{
> > + ?? ??size_t reg = addr & ~0x07;
> > + ?? ??uint64_t *base = (uint64_t *) &st->mmio_buf[reg];
> 
> This is still buggy.
> 
> > + ?? ??uint64_t val = le64_to_cpu(*base);

mmio_buf is always LE, so a BE host will have *base in reversed
byteorder. But look at the next line, where I did the le64_to_cpu().
That should swap the bytes on a BE host, yielding the correct byteorder.

On a LE host, *base is right the first time and le64_to_cpu() is a nop.

In any case, I only use 'val', not '*base' directly. I suppose it could
be rewritten for clarity (i.e. ditch 'base').

Do you still think it's wrong? Or is it for another reason?


	Thanks,
	Eduard