Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
>>> On 23.02.15 at 16:33, wrote: > On Mon, 2015-02-23 at 09:37 +, Jan Beulich wrote: >> >>> On 20.02.15 at 18:46, wrote: >> > On 20/02/15 17:03, Ian Campbell wrote: >> >> On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: >> >> >> >> Subject: "release the DT devices assigned to a guest earlier" >> >> >> >>> The toolstack may not have deassign every device used by a guest. >> >> >> >> "deassigned" >> >> >> >>> Therefore we have to go through the device list and removing them before >> >> >> >> "and remove them" >> >> >> >>> asking the IOMMU drivers to release memory for this domain. >> >>> >> >>> This can be done by moving the call to the release function when we >> >>> relinquish the resources. The IOMMU part will be destroyed later when >> >>> the domain is freed. >> >>> >> >>> Signed-off-by: Julien Grall >> >>> Cc: Jan Beulich >> >> >> >> Acked-by: Ian Campbell >> >> >> >> I'd like to hear Jan's opinion though. >> > >> > FWIW, this is how it's done for PCI. >> >> Right, IOW I agree to the idea of the patch. > > Thanks. If Julien could record this in the form of an ack that would > save me remembering you've said so (otherwise it could just go as a > comment after the --- I suppose). That's fine with me, I just didn't say so because it seemed a bit odd on an ARM-only change. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
On Mon, 2015-02-23 at 09:37 +, Jan Beulich wrote: > >>> On 20.02.15 at 18:46, wrote: > > On 20/02/15 17:03, Ian Campbell wrote: > >> On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: > >> > >> Subject: "release the DT devices assigned to a guest earlier" > >> > >>> The toolstack may not have deassign every device used by a guest. > >> > >> "deassigned" > >> > >>> Therefore we have to go through the device list and removing them before > >> > >> "and remove them" > >> > >>> asking the IOMMU drivers to release memory for this domain. > >>> > >>> This can be done by moving the call to the release function when we > >>> relinquish the resources. The IOMMU part will be destroyed later when > >>> the domain is freed. > >>> > >>> Signed-off-by: Julien Grall > >>> Cc: Jan Beulich > >> > >> Acked-by: Ian Campbell > >> > >> I'd like to hear Jan's opinion though. > > > > FWIW, this is how it's done for PCI. > > Right, IOW I agree to the idea of the patch. Thanks. If Julien could record this in the form of an ack that would save me remembering you've said so (otherwise it could just go as a comment after the --- I suppose). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
>>> On 20.02.15 at 18:46, wrote: > On 20/02/15 17:03, Ian Campbell wrote: >> On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: >> >> Subject: "release the DT devices assigned to a guest earlier" >> >>> The toolstack may not have deassign every device used by a guest. >> >> "deassigned" >> >>> Therefore we have to go through the device list and removing them before >> >> "and remove them" >> >>> asking the IOMMU drivers to release memory for this domain. >>> >>> This can be done by moving the call to the release function when we >>> relinquish the resources. The IOMMU part will be destroyed later when >>> the domain is freed. >>> >>> Signed-off-by: Julien Grall >>> Cc: Jan Beulich >> >> Acked-by: Ian Campbell >> >> I'd like to hear Jan's opinion though. > > FWIW, this is how it's done for PCI. Right, IOW I agree to the idea of the patch. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
On 20/02/15 17:03, Ian Campbell wrote: > On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: > > Subject: "release the DT devices assigned to a guest earlier" > >> The toolstack may not have deassign every device used by a guest. > > "deassigned" > >> Therefore we have to go through the device list and removing them before > > "and remove them" > >> asking the IOMMU drivers to release memory for this domain. >> >> This can be done by moving the call to the release function when we >> relinquish the resources. The IOMMU part will be destroyed later when >> the domain is freed. >> >> Signed-off-by: Julien Grall >> Cc: Jan Beulich > > Acked-by: Ian Campbell > > I'd like to hear Jan's opinion though. FWIW, this is how it's done for PCI. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: Subject: "release the DT devices assigned to a guest earlier" > The toolstack may not have deassign every device used by a guest. "deassigned" > Therefore we have to go through the device list and removing them before "and remove them" > asking the IOMMU drivers to release memory for this domain. > > This can be done by moving the call to the release function when we > relinquish the resources. The IOMMU part will be destroyed later when > the domain is freed. > > Signed-off-by: Julien Grall > Cc: Jan Beulich Acked-by: Ian Campbell I'd like to hear Jan's opinion though. > > --- > Changes in v3: > - Patch added. Superseed the patch "xen/passthrough: call > arch_iommu_domain_destroy before calling iommu teardown" in > the previous patch series. > --- > xen/arch/arm/domain.c | 4 > xen/drivers/passthrough/arm/iommu.c | 1 - > xen/drivers/passthrough/device_tree.c | 5 - > xen/include/xen/iommu.h | 2 +- > 4 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 6e56665..d85748a 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -772,6 +772,10 @@ int domain_relinquish_resources(struct domain *d) > switch ( d->arch.relmem ) > { > case RELMEM_not_started: > +ret = iommu_release_dt_devices(d); > +if ( ret ) > +return ret; > + > d->arch.relmem = RELMEM_xen; > /* Falltrough */ > > diff --git a/xen/drivers/passthrough/arm/iommu.c > b/xen/drivers/passthrough/arm/iommu.c > index 5870aef..8223a39 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -66,7 +66,6 @@ int arch_iommu_domain_init(struct domain *d) > > void arch_iommu_domain_destroy(struct domain *d) > { > -iommu_dt_domain_destroy(d); > } > > int arch_iommu_populate_page_table(struct domain *d) > diff --git a/xen/drivers/passthrough/device_tree.c > b/xen/drivers/passthrough/device_tree.c > index 88e496e..e7eb34f 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -97,7 +97,7 @@ int iommu_dt_domain_init(struct domain *d) > return 0; > } > > -void iommu_dt_domain_destroy(struct domain *d) > +int iommu_release_dt_devices(struct domain *d) > { > struct hvm_iommu *hd = domain_hvm_iommu(d); > struct dt_device_node *dev, *_dev; > @@ -109,5 +109,8 @@ void iommu_dt_domain_destroy(struct domain *d) > if ( rc ) > dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n", > dt_node_full_name(dev), d->domain_id); > +return rc; > } > + > +return 0; > } > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index c146ee4..d03df14 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, > struct msi_msg *msg); > int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_dt_domain_init(struct domain *d); > -void iommu_dt_domain_destroy(struct domain *d); > +int iommu_release_dt_devices(struct domain *d); > > #endif /* HAS_DEVICE_TREE */ > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
On Tue, 13 Jan 2015, Julien Grall wrote: > The toolstack may not have deassign every device used by a guest. > Therefore we have to go through the device list and removing them before > asking the IOMMU drivers to release memory for this domain. > > This can be done by moving the call to the release function when we > relinquish the resources. The IOMMU part will be destroyed later when > the domain is freed. > > Signed-off-by: Julien Grall > Cc: Jan Beulich > > Changes in v3: > - Patch added. Superseed the patch "xen/passthrough: call > arch_iommu_domain_destroy before calling iommu teardown" in > the previous patch series. Acked-by: Stefano Stabellini > xen/arch/arm/domain.c | 4 > xen/drivers/passthrough/arm/iommu.c | 1 - > xen/drivers/passthrough/device_tree.c | 5 - > xen/include/xen/iommu.h | 2 +- > 4 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 6e56665..d85748a 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -772,6 +772,10 @@ int domain_relinquish_resources(struct domain *d) > switch ( d->arch.relmem ) > { > case RELMEM_not_started: > +ret = iommu_release_dt_devices(d); > +if ( ret ) > +return ret; > + > d->arch.relmem = RELMEM_xen; > /* Falltrough */ > > diff --git a/xen/drivers/passthrough/arm/iommu.c > b/xen/drivers/passthrough/arm/iommu.c > index 5870aef..8223a39 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -66,7 +66,6 @@ int arch_iommu_domain_init(struct domain *d) > > void arch_iommu_domain_destroy(struct domain *d) > { > -iommu_dt_domain_destroy(d); > } > > int arch_iommu_populate_page_table(struct domain *d) > diff --git a/xen/drivers/passthrough/device_tree.c > b/xen/drivers/passthrough/device_tree.c > index 88e496e..e7eb34f 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -97,7 +97,7 @@ int iommu_dt_domain_init(struct domain *d) > return 0; > } > > -void iommu_dt_domain_destroy(struct domain *d) > +int iommu_release_dt_devices(struct domain *d) > { > struct hvm_iommu *hd = domain_hvm_iommu(d); > struct dt_device_node *dev, *_dev; > @@ -109,5 +109,8 @@ void iommu_dt_domain_destroy(struct domain *d) > if ( rc ) > dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n", > dt_node_full_name(dev), d->domain_id); > +return rc; > } > + > +return 0; > } > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index c146ee4..d03df14 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, > struct msi_msg *msg); > int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_dt_domain_init(struct domain *d); > -void iommu_dt_domain_destroy(struct domain *d); > +int iommu_release_dt_devices(struct domain *d); > > #endif /* HAS_DEVICE_TREE */ > > -- > 2.1.4 > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
>>> On 20.01.15 at 15:30, wrote: > Hi Jan, > > On 20/01/15 09:19, Jan Beulich wrote: > On 13.01.15 at 15:25, wrote: >>> --- a/xen/include/xen/iommu.h >>> +++ b/xen/include/xen/iommu.h >>> @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, > struct msi_msg *msg); >>> int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); >>> int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); >>> int iommu_dt_domain_init(struct domain *d); >>> -void iommu_dt_domain_destroy(struct domain *d); >>> +int iommu_release_dt_devices(struct domain *d); >> >> I don't think you really need an ack from me on this trivial change, >> but in case you feel otherwise, don't hesitate to add it. > > I wasn't sure if I add to cc you. You were in the list of maintainers > with scripts/get_maintainers.pl because the include is part of the > "IOMMU VENDOR INDEPENDENT CODE". Which is fine - I merely wanted to point out that a trivial prototype adjustment can imo go in without requiring an ack. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
Hi Jan, On 20/01/15 09:19, Jan Beulich wrote: On 13.01.15 at 15:25, wrote: >> --- a/xen/include/xen/iommu.h >> +++ b/xen/include/xen/iommu.h >> @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, >> struct msi_msg *msg); >> int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); >> int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); >> int iommu_dt_domain_init(struct domain *d); >> -void iommu_dt_domain_destroy(struct domain *d); >> +int iommu_release_dt_devices(struct domain *d); > > I don't think you really need an ack from me on this trivial change, > but in case you feel otherwise, don't hesitate to add it. I wasn't sure if I add to cc you. You were in the list of maintainers with scripts/get_maintainers.pl because the include is part of the "IOMMU VENDOR INDEPENDENT CODE". Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
>>> On 13.01.15 at 15:25, wrote: > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, > struct msi_msg *msg); > int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_dt_domain_init(struct domain *d); > -void iommu_dt_domain_destroy(struct domain *d); > +int iommu_release_dt_devices(struct domain *d); I don't think you really need an ack from me on this trivial change, but in case you feel otherwise, don't hesitate to add it. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 17/24] xen/passthrough: arm: release earlier the DT devices assigned to a guest
The toolstack may not have deassign every device used by a guest. Therefore we have to go through the device list and removing them before asking the IOMMU drivers to release memory for this domain. This can be done by moving the call to the release function when we relinquish the resources. The IOMMU part will be destroyed later when the domain is freed. Signed-off-by: Julien Grall Cc: Jan Beulich --- Changes in v3: - Patch added. Superseed the patch "xen/passthrough: call arch_iommu_domain_destroy before calling iommu teardown" in the previous patch series. --- xen/arch/arm/domain.c | 4 xen/drivers/passthrough/arm/iommu.c | 1 - xen/drivers/passthrough/device_tree.c | 5 - xen/include/xen/iommu.h | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 6e56665..d85748a 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -772,6 +772,10 @@ int domain_relinquish_resources(struct domain *d) switch ( d->arch.relmem ) { case RELMEM_not_started: +ret = iommu_release_dt_devices(d); +if ( ret ) +return ret; + d->arch.relmem = RELMEM_xen; /* Falltrough */ diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c index 5870aef..8223a39 100644 --- a/xen/drivers/passthrough/arm/iommu.c +++ b/xen/drivers/passthrough/arm/iommu.c @@ -66,7 +66,6 @@ int arch_iommu_domain_init(struct domain *d) void arch_iommu_domain_destroy(struct domain *d) { -iommu_dt_domain_destroy(d); } int arch_iommu_populate_page_table(struct domain *d) diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c index 88e496e..e7eb34f 100644 --- a/xen/drivers/passthrough/device_tree.c +++ b/xen/drivers/passthrough/device_tree.c @@ -97,7 +97,7 @@ int iommu_dt_domain_init(struct domain *d) return 0; } -void iommu_dt_domain_destroy(struct domain *d) +int iommu_release_dt_devices(struct domain *d) { struct hvm_iommu *hd = domain_hvm_iommu(d); struct dt_device_node *dev, *_dev; @@ -109,5 +109,8 @@ void iommu_dt_domain_destroy(struct domain *d) if ( rc ) dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n", dt_node_full_name(dev), d->domain_id); +return rc; } + +return 0; } diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index c146ee4..d03df14 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -117,7 +117,7 @@ void iommu_read_msi_from_ire(struct msi_desc *msi_desc, struct msi_msg *msg); int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); int iommu_dt_domain_init(struct domain *d); -void iommu_dt_domain_destroy(struct domain *d); +int iommu_release_dt_devices(struct domain *d); #endif /* HAS_DEVICE_TREE */ -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel