From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 09/15] xen/passthrough: iommu: Introduce arch specific code Date: Mon, 24 Feb 2014 13:33:44 +0000 Message-ID: <530B4A38.5070008@linaro.org> References: <1393193792-20008-1-git-send-email-julien.grall@linaro.org> <1393193792-20008-10-git-send-email-julien.grall@linaro.org> <530B3083020000780011EAB1@nat28.tlf.novell.com> <530B41D5.8070803@linaro.org> <530B544F020000780011EC94@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WHvfM-0001Qc-EG for xen-devel@lists.xenproject.org; Mon, 24 Feb 2014 13:33:52 +0000 Received: by mail-ea0-f180.google.com with SMTP id o10so3060733eaj.25 for ; Mon, 24 Feb 2014 05:33:50 -0800 (PST) In-Reply-To: <530B544F020000780011EC94@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , ian.campbell@citrix.com, Shane Wang , Joseph Cihula , tim@xen.org, stefano.stabellini@citrix.com, Suravee Suthikulpanit , xen-devel@lists.xenproject.org, Gang Wei , Xiantao Zhang List-Id: xen-devel@lists.xenproject.org On 02/24/2014 01:16 PM, Jan Beulich wrote: >>>> On 24.02.14 at 13:57, Julien Grall 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. I didn't want to repeat that I said for the previous field. ARM doesn't have a generic IOMMU driver (or two) as x86. Basically every vendor can decide to design it's own IOMMU. To avoid to have a big structure with some fields used only by one driver, every IOMMU ARM drivers as its own private structure. IHMO, I think every fields stored in the generic hvm_iommu structure should be initialized by generic code, not by a specific driver. -- Julien Grall