| Message ID | 1463168217-16080-3-git-send-email-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: suravee.suthikulpanit@amd.com > [mailto:suravee.suthikulpanit@amd.com] > Sent: 13 May 2016 20:37 > To: xen-devel@lists.xen.org; George Dunlap; jbeulich@suse.com > Cc: Paul Durrant; Suravee Suthikulpanit; Suravee Suthikulpanit > Subject: [PATCH V2 2/2] svm: iommu: Only call guest_iommu_init() after > initialized HVM domain > > From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > > The guest_iommu_init() is currently called by the following code path: > > arch/x86/domain.c: arch_domain_create() > ]- drivers/passthrough/iommu.c: iommu_domain_init() > |- drivers/passthrough/amd/pci_amd_iommu.c: > amd_iommu_domain_init(); > |- drivers/passthrough/amd/iommu_guest.c: guest_iommu_init() > > At this point, the hvm_domain_initialised() has not been > called. So register_mmio_handler(), in guest_iommu_init(), silently fails. > This patch moves the call to guest_iommu_init/destroy() into > the svm_domain_intialise/_destroy() instead. > That seems wrong. You're taking a call that currently comes via a jump table, i.e. an abstraction layer, and calling it directly. Is it possible, instead, to move the call to iommu_domain_init() later in arch_domain_create()? It seems odd, to me at least, that it's done before hvm_domain_initialise() anyway. Paul > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > xen/arch/x86/hvm/svm/svm.c | 10 ++++++++++ > xen/drivers/passthrough/amd/iommu_guest.c | 6 ++++++ > xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 ---- > 3 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index e62dfa1..0c4affc 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -45,6 +45,7 @@ > #include <asm/hvm/support.h> > #include <asm/hvm/io.h> > #include <asm/hvm/emulate.h> > +#include <asm/hvm/svm/amd-iommu-proto.h> > #include <asm/hvm/svm/asid.h> > #include <asm/hvm/svm/svm.h> > #include <asm/hvm/svm/vmcb.h> > @@ -1176,11 +1177,20 @@ void svm_host_osvw_init() > > static int svm_domain_initialise(struct domain *d) > { > + if ( is_hvm_domain(d) ) > + /* > + * This requires the hvm domain to be > + * initialized first. > + */ > + return guest_iommu_init(d); > + > return 0; > } > > static void svm_domain_destroy(struct domain *d) > { > + if ( is_hvm_domain(d) ) > + guest_iommu_destroy(d); > } > > static int svm_vcpu_initialise(struct vcpu *v) > diff --git a/xen/drivers/passthrough/amd/iommu_guest.c > b/xen/drivers/passthrough/amd/iommu_guest.c > index b4e75ac..9f26765 100644 > --- a/xen/drivers/passthrough/amd/iommu_guest.c > +++ b/xen/drivers/passthrough/amd/iommu_guest.c > @@ -891,6 +891,12 @@ int guest_iommu_init(struct domain* d) > !has_viommu(d) ) > return 0; > > + if ( d->arch.hvm_domain.io_handler == NULL ) > + { > + AMD_IOMMU_DEBUG("Error: uninitalized hvm io handler\n"); > + return 1; > + } > + > iommu = xzalloc(struct guest_iommu); > if ( !iommu ) > { > diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c > b/xen/drivers/passthrough/amd/pci_amd_iommu.c > index c1c0b6b..f791618 100644 > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c > @@ -274,9 +274,6 @@ static int amd_iommu_domain_init(struct domain *d) > hd->arch.paging_mode = is_hvm_domain(d) ? > IOMMU_PAGING_MODE_LEVEL_2 : > get_paging_mode(max_page); > - > - guest_iommu_init(d); > - > return 0; > } > > @@ -476,7 +473,6 @@ static void deallocate_iommu_page_tables(struct > domain *d) > > static void amd_iommu_domain_destroy(struct domain *d) > { > - guest_iommu_destroy(d); > deallocate_iommu_page_tables(d); > amd_iommu_flush_all_pages(d); > } > -- > 1.9.1
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index e62dfa1..0c4affc 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -45,6 +45,7 @@ #include <asm/hvm/support.h> #include <asm/hvm/io.h> #include <asm/hvm/emulate.h> +#include <asm/hvm/svm/amd-iommu-proto.h> #include <asm/hvm/svm/asid.h> #include <asm/hvm/svm/svm.h> #include <asm/hvm/svm/vmcb.h> @@ -1176,11 +1177,20 @@ void svm_host_osvw_init() static int svm_domain_initialise(struct domain *d) { + if ( is_hvm_domain(d) ) + /* + * This requires the hvm domain to be + * initialized first. + */ + return guest_iommu_init(d); + return 0; } static void svm_domain_destroy(struct domain *d) { + if ( is_hvm_domain(d) ) + guest_iommu_destroy(d); } static int svm_vcpu_initialise(struct vcpu *v) diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c index b4e75ac..9f26765 100644 --- a/xen/drivers/passthrough/amd/iommu_guest.c +++ b/xen/drivers/passthrough/amd/iommu_guest.c @@ -891,6 +891,12 @@ int guest_iommu_init(struct domain* d) !has_viommu(d) ) return 0; + if ( d->arch.hvm_domain.io_handler == NULL ) + { + AMD_IOMMU_DEBUG("Error: uninitalized hvm io handler\n"); + return 1; + } + iommu = xzalloc(struct guest_iommu); if ( !iommu ) { diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index c1c0b6b..f791618 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -274,9 +274,6 @@ static int amd_iommu_domain_init(struct domain *d) hd->arch.paging_mode = is_hvm_domain(d) ? IOMMU_PAGING_MODE_LEVEL_2 : get_paging_mode(max_page); - - guest_iommu_init(d); - return 0; } @@ -476,7 +473,6 @@ static void deallocate_iommu_page_tables(struct domain *d) static void amd_iommu_domain_destroy(struct domain *d) { - guest_iommu_destroy(d); deallocate_iommu_page_tables(d); amd_iommu_flush_all_pages(d); }