Re: [Qemu-devel] Re: [RFC PATCH] AMD IOMMU emulation

[Blue Swirl]

On Mon, May 24, 2010 at 3:40 PM, Joerg Roedel <address@hidden> wrote:
> Hi Eduard,
>
> On Thu, May 20, 2010 at 04:50:07PM +0300, Eduard - Gabriel Munteanu wrote:
>> +  --enable-amd-iommu-emul) amd_iommu="yes"
>> +  ;;
>
> A compile-time option is a good idea.
>
>> +/* MMIO registers */
>> +#define MMIO_DEVICE_TABLE       0x0000
>> +#define MMIO_COMMAND_BASE       0x0008
>> +#define MMIO_EVENT_BASE         0x0010
>> +#define MMIO_CONTROL            0x0018
>> +#define MMIO_EXCL_BASE          0x0020
>> +#define MMIO_EXCL_LIMIT         0x0028
>> +#define MMIO_COMMAND_HEAD       0x2000
>> +#define MMIO_COMMAND_TAIL       0x2008
>> +#define MMIO_EVENT_HEAD         0x2010
>> +#define MMIO_EVENT_TAIL         0x2018
>> +#define MMIO_STATUS             0x2020
>> +
>> +#define MMIO_SIZE               0x2028
>
> This size should be a power-of-two value. In this case probably 0x4000.

Not really, the devices can reserve regions of any size. There were
some implementation deficiencies in earlier versions of QEMU, where
the whole page would be reserved anyway, but this limitation has been
removed long time ago.

>> +#define MMIO_DEVTAB_SIZE_MASK   ((1UL << 12) - 1)
>> +#define MMIO_DEVTAB_BASE_MASK   (((1UL << 52) - 1) & ~MMIO_DEVTAB_SIZE_MASK)
>
> You must use ULL to be 32bit safe. This is also true for the defines
> below.
>
> [...]
>
> Otherwise the code looks good so far. Seems like the next step should be
> some work on a qemu dma-layer where you can hook the translation into.
>
>        Joerg
>
>
>