Re: [Xen-devel] [PATCH v2 09/15] xen/passthrough: iommu: Introduce arch specific code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.02.14 at 13:57, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>>>  struct hvm_iommu {
>>> -    u64 pgd_maddr;                 /* io page directory machine address */
>>> -    spinlock_t mapping_lock;       /* io page table lock */
>>> -    int agaw;     /* adjusted guest address width, 0 is level 2 30-bit */
>>> -    struct list_head g2m_ioport_list;  /* guest to machine ioport mapping 
>>> */
>>> -    u64 iommu_bitmap;              /* bitmap of iommu(s) that the domain 
>>> uses */
>>> -    struct list_head mapped_rmrrs;
>>> -
>>> -    /* amd iommu support */
>>> -    int domain_id;
>> 
>> At the very least this field doesn't look all that architecture specific,
>> even if it might only be used on x86/AMD right now.
> 
> On ARM, each IOMMU will have it's own private data stored in arch.priv.
> I don't think domain_id will be used as the driver can directly use
> d->domain_id.
> 
> I gave a look on AMD IOMMU drivers, and in a same function they mixed
> d->domain_id and domain_hvm_iommu(d)->arch.domain_id. The latter one has
> been initialized to d->domain_id in amd_iommu_domain_init.
> 
> I think, we can even remove this field for x86...

As Andrew suggested too.

>>> -    int paging_mode;
>> 
>> The same might go for this one.
> 
> There is only one paging mode on ARM.

But please make your changes here not in the spirit of fitting in ARM,
but to make the code reasonable architecture clean. Which means
that fields having a purpose outside of x86 should remain common,
instead of making x86-specific everything that ARM (currently) has
no use for.

Jan


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