* [PATCH 0/3] xen/arm: vgic-v3: Bug fixes @ 2018-09-04 19:21 Julien Grall 2018-09-04 19:21 ` [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall ` (3 more replies) 0 siblings, 4 replies; 24+ messages in thread From: Julien Grall @ 2018-09-04 19:21 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, sstabellini, shameerali.kolothum.thodi Hi all, The first 2 patches of the series are meant to address Dom0 boot failure when on GICv4 platforms as well as when the number of vCPUs is not equal to the numbers of pCPUs. They should be backported up Xen 4.8. Patch #3 should address failure when failing to create guest. ITS is still under EXPERT, so I would not consider it as backport. Cheers, Julien Grall (3): [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information xen/arm: vgic-v3: Don't create empty re-distributor regions xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent xen/arch/arm/gic-v3.c | 10 ++++++++-- xen/arch/arm/vgic-v3-its.c | 4 ++++ xen/arch/arm/vgic-v3.c | 40 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 4 deletions(-) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-04 19:21 [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Julien Grall @ 2018-09-04 19:21 ` Julien Grall 2018-09-04 19:35 ` Julien Grall 2018-09-04 19:21 ` [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-04 19:21 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, sstabellini, shameerali.kolothum.thodi, Andrew Cooper A follow-up patch will require to know the number of vCPUs when initializating the vGICv3 domain structure. However this information is not available at domain creation. This is only known once XEN_DOMCTL_max_vpus is called for that domain. In order to get the max vCPUs around, delay the domain part of the vGIC v3 initialization until the first vCPU of the domain is initialized. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> This is nasty but I can't find a better way for Xen 4.11 and older. This is not necessary for unstable as the number of vCPUs is known at domain creation. Andrew, I have CCed you to know whether you have a better idea where to place this call on Xen 4.11 and older. --- xen/arch/arm/vgic-v3.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 4b42739a52..df1bab3a35 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1573,9 +1573,11 @@ static const struct mmio_handler_ops vgic_distr_mmio_handler = { .write = vgic_v3_distr_mmio_write, }; +static int vgic_v3_real_domain_init(struct domain *d); + static int vgic_v3_vcpu_init(struct vcpu *v) { - int i; + int i, rc; paddr_t rdist_base; struct vgic_rdist_region *region; unsigned int last_cpu; @@ -1584,6 +1586,19 @@ static int vgic_v3_vcpu_init(struct vcpu *v) struct domain *d = v->domain; /* + * This is the earliest place where the number of vCPUs is + * known. This is required to initialize correctly the vGIC v3 + * domain structure. We only to do that when vCPU 0 is + * initilialized. + */ + if ( v->vcpu_id == 0 ) + { + rc = vgic_v3_real_domain_init(d); + if ( rc ) + return rc; + } + + /* * Find the region where the re-distributor lives. For this purpose, * we look one region ahead as we have only the first CPU in hand. */ @@ -1641,7 +1656,7 @@ static inline unsigned int vgic_v3_rdist_count(struct domain *d) GUEST_GICV3_RDIST_REGIONS; } -static int vgic_v3_domain_init(struct domain *d) +static int vgic_v3_real_domain_init(struct domain *d) { struct vgic_rdist_region *rdist_regions; int rdist_count, i, ret; @@ -1733,6 +1748,16 @@ static int vgic_v3_domain_init(struct domain *d) return 0; } +static int vgic_v3_domain_init(struct domain *d) +{ + /* + * The domain initialization for vGIC v3 is delayed until the first vCPU + * is created. This because the initialization may require to know the + * number of vCPUs that is not known when creating the domain. + */ + return 0; +} + static void vgic_v3_domain_free(struct domain *d) { vgic_v3_its_free_domain(d); -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-04 19:21 ` [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall @ 2018-09-04 19:35 ` Julien Grall 2018-09-04 19:53 ` Andrew Cooper 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-04 19:35 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, sstabellini, shameerali.kolothum.thodi, Andrew Cooper Hi, On 09/04/2018 08:21 PM, Julien Grall wrote: > A follow-up patch will require to know the number of vCPUs when > initializating the vGICv3 domain structure. However this information is > not available at domain creation. This is only known once > XEN_DOMCTL_max_vpus is called for that domain. > > In order to get the max vCPUs around, delay the domain part of the vGIC > v3 initialization until the first vCPU of the domain is initialized. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > --- > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > This is nasty but I can't find a better way for Xen 4.11 and older. This > is not necessary for unstable as the number of vCPUs is known at domain > creation. > > Andrew, I have CCed you to know whether you have a better idea where to > place this call on Xen 4.11 and older. I just noticed that d->max_vcpus is initialized after arch_domain_create. So without this patch on Xen 4.12, it will not work. This is getting nastier because arch_domain_init is the one initialize the value returned by dom0_max_vcpus. So I am not entirely sure what to do here. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-04 19:35 ` Julien Grall @ 2018-09-04 19:53 ` Andrew Cooper 2018-09-05 13:25 ` Julien Grall 2018-09-25 20:45 ` Stefano Stabellini 0 siblings, 2 replies; 24+ messages in thread From: Andrew Cooper @ 2018-09-04 19:53 UTC (permalink / raw) To: Julien Grall, xen-devel Cc: andre.przywara, sstabellini, shameerali.kolothum.thodi On 04/09/18 20:35, Julien Grall wrote: > Hi, > > On 09/04/2018 08:21 PM, Julien Grall wrote: >> A follow-up patch will require to know the number of vCPUs when >> initializating the vGICv3 domain structure. However this information is >> not available at domain creation. This is only known once >> XEN_DOMCTL_max_vpus is called for that domain. >> >> In order to get the max vCPUs around, delay the domain part of the vGIC >> v3 initialization until the first vCPU of the domain is initialized. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> >> --- >> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >> >> This is nasty but I can't find a better way for Xen 4.11 and older. This >> is not necessary for unstable as the number of vCPUs is known at domain >> creation. >> >> Andrew, I have CCed you to know whether you have a better idea where to >> place this call on Xen 4.11 and older. > > I just noticed that d->max_vcpus is initialized after > arch_domain_create. So without this patch on Xen 4.12, it will not work. > > This is getting nastier because arch_domain_init is the one initialize > the value returned by dom0_max_vcpus. So I am not entirely sure what > to do here. The positioning after arch_domain_create() is unfortunate, but I couldn’t manage better with ARM's current behaviour and Jan's insistence that the allocation of d->vcpu was common. I'd prefer if the dependency could be broken and the allocation moved earlier. One option might be to have an arch_check_domainconfig() (or similar?) which is called very early on and can sanity check the values, including cross-checking the vgic and max_vcpus settings? It could even be responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct real value. As for your patch here, its a gross hack, but its probably the best which can be done. I have to admit that I'm surprised/concerned that Xen has lasted this long with such a fundamental gap in domain creation. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-04 19:53 ` Andrew Cooper @ 2018-09-05 13:25 ` Julien Grall 2018-09-25 20:45 ` Stefano Stabellini 1 sibling, 0 replies; 24+ messages in thread From: Julien Grall @ 2018-09-05 13:25 UTC (permalink / raw) To: Andrew Cooper, xen-devel Cc: andre.przywara, sstabellini, shameerali.kolothum.thodi Hi Andrew, On 09/04/2018 08:53 PM, Andrew Cooper wrote: > On 04/09/18 20:35, Julien Grall wrote: >> Hi, >> >> On 09/04/2018 08:21 PM, Julien Grall wrote: >>> A follow-up patch will require to know the number of vCPUs when >>> initializating the vGICv3 domain structure. However this information is >>> not available at domain creation. This is only known once >>> XEN_DOMCTL_max_vpus is called for that domain. >>> >>> In order to get the max vCPUs around, delay the domain part of the vGIC >>> v3 initialization until the first vCPU of the domain is initialized. >>> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>> >>> --- >>> >>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>> >>> This is nasty but I can't find a better way for Xen 4.11 and older. This >>> is not necessary for unstable as the number of vCPUs is known at domain >>> creation. >>> >>> Andrew, I have CCed you to know whether you have a better idea where to >>> place this call on Xen 4.11 and older. >> >> I just noticed that d->max_vcpus is initialized after >> arch_domain_create. So without this patch on Xen 4.12, it will not work. >> >> This is getting nastier because arch_domain_init is the one initialize >> the value returned by dom0_max_vcpus. So I am not entirely sure what >> to do here. > > The positioning after arch_domain_create() is unfortunate, but I > couldn’t manage better with ARM's current behaviour and Jan's insistence > that the allocation of d->vcpu was common. I'd prefer if the dependency > could be broken and the allocation moved earlier. > > One option might be to have an arch_check_domainconfig() (or similar?) > which is called very early on and can sanity check the values, including > cross-checking the vgic and max_vcpus settings? It could even be > responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct > real value. I think it is doable without too much trouble on Arm. I can have a look at this for Xen 4.12 once this series is merged. > > As for your patch here, its a gross hack, but its probably the best > which can be done. I have to admit that I'm surprised/concerned that > Xen has lasted this long with such a fundamental gap in domain creation. I am surprised too. Hopefully your rework will make everything easier to use. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-04 19:53 ` Andrew Cooper 2018-09-05 13:25 ` Julien Grall @ 2018-09-25 20:45 ` Stefano Stabellini 2018-09-26 20:14 ` Julien Grall 1 sibling, 1 reply; 24+ messages in thread From: Stefano Stabellini @ 2018-09-25 20:45 UTC (permalink / raw) To: Andrew Cooper Cc: xen-devel, Julien Grall, sstabellini, shameerali.kolothum.thodi, andre.przywara [-- Attachment #1: Type: TEXT/PLAIN, Size: 2407 bytes --] On Tue, 4 Sep 2018, Andrew Cooper wrote: > On 04/09/18 20:35, Julien Grall wrote: > > Hi, > > > > On 09/04/2018 08:21 PM, Julien Grall wrote: > >> A follow-up patch will require to know the number of vCPUs when > >> initializating the vGICv3 domain structure. However this information is > >> not available at domain creation. This is only known once > >> XEN_DOMCTL_max_vpus is called for that domain. > >> > >> In order to get the max vCPUs around, delay the domain part of the vGIC > >> v3 initialization until the first vCPU of the domain is initialized. > >> > >> Signed-off-by: Julien Grall <julien.grall@arm.com> > >> > >> --- > >> > >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> > >> > >> This is nasty but I can't find a better way for Xen 4.11 and older. This > >> is not necessary for unstable as the number of vCPUs is known at domain > >> creation. > >> > >> Andrew, I have CCed you to know whether you have a better idea where to > >> place this call on Xen 4.11 and older. > > > > I just noticed that d->max_vcpus is initialized after > > arch_domain_create. So without this patch on Xen 4.12, it will not work. > > > > This is getting nastier because arch_domain_init is the one initialize > > the value returned by dom0_max_vcpus. So I am not entirely sure what > > to do here. > > The positioning after arch_domain_create() is unfortunate, but I > couldn’t manage better with ARM's current behaviour and Jan's insistence > that the allocation of d->vcpu was common. I'd prefer if the dependency > could be broken and the allocation moved earlier. > > One option might be to have an arch_check_domainconfig() (or similar?) > which is called very early on and can sanity check the values, including > cross-checking the vgic and max_vcpus settings? It could even be > responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct > real value. > > As for your patch here, its a gross hack, but its probably the best > which can be done. *Sighs* If that is what we have to do, it is as ugly as hell, but that is what we'll do. My only suggestion to marginally improve it would be instead of: > + if ( v->vcpu_id == 0 ) > + { > + rc = vgic_v3_real_domain_init(d); > + if ( rc ) > + return rc; > + } to check on d->arch.vgic.rdist_regions instead: if ( d->arch.vgic.rdist_regions == NULL ) { // initialize domain [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-25 20:45 ` Stefano Stabellini @ 2018-09-26 20:14 ` Julien Grall 2018-09-27 23:11 ` Stefano Stabellini 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-26 20:14 UTC (permalink / raw) To: Stefano Stabellini, Andrew Cooper Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara Hi Stefano, On 09/25/2018 09:45 PM, Stefano Stabellini wrote: > On Tue, 4 Sep 2018, Andrew Cooper wrote: >> On 04/09/18 20:35, Julien Grall wrote: >>> Hi, >>> >>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>> A follow-up patch will require to know the number of vCPUs when >>>> initializating the vGICv3 domain structure. However this information is >>>> not available at domain creation. This is only known once >>>> XEN_DOMCTL_max_vpus is called for that domain. >>>> >>>> In order to get the max vCPUs around, delay the domain part of the vGIC >>>> v3 initialization until the first vCPU of the domain is initialized. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>> >>>> --- >>>> >>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>> >>>> This is nasty but I can't find a better way for Xen 4.11 and older. This >>>> is not necessary for unstable as the number of vCPUs is known at domain >>>> creation. >>>> >>>> Andrew, I have CCed you to know whether you have a better idea where to >>>> place this call on Xen 4.11 and older. >>> >>> I just noticed that d->max_vcpus is initialized after >>> arch_domain_create. So without this patch on Xen 4.12, it will not work. >>> >>> This is getting nastier because arch_domain_init is the one initialize >>> the value returned by dom0_max_vcpus. So I am not entirely sure what >>> to do here. >> >> The positioning after arch_domain_create() is unfortunate, but I >> couldn’t manage better with ARM's current behaviour and Jan's insistence >> that the allocation of d->vcpu was common. I'd prefer if the dependency >> could be broken and the allocation moved earlier. >> >> One option might be to have an arch_check_domainconfig() (or similar?) >> which is called very early on and can sanity check the values, including >> cross-checking the vgic and max_vcpus settings? It could even be >> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct >> real value. >> >> As for your patch here, its a gross hack, but its probably the best >> which can be done. > > *Sighs* > If that is what we have to do, it is as ugly as hell, but that is what > we'll do. This is the best we can do with the current code base. I think it would be worth reworking the code to make it nicer. I will add it in my TODO list. > > My only suggestion to marginally improve it would be instead of: > >> + if ( v->vcpu_id == 0 ) >> + { >> + rc = vgic_v3_real_domain_init(d); >> + if ( rc ) >> + return rc; >> + } > > to check on d->arch.vgic.rdist_regions instead: > > if ( d->arch.vgic.rdist_regions == NULL ) > { > // initialize domain I would prefer to keep v->vcpu_id == 0 just in case we end up to re-order the allocation in the future. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-26 20:14 ` Julien Grall @ 2018-09-27 23:11 ` Stefano Stabellini 2018-09-28 20:35 ` Julien Grall 0 siblings, 1 reply; 24+ messages in thread From: Stefano Stabellini @ 2018-09-27 23:11 UTC (permalink / raw) To: Julien Grall Cc: Andrew Cooper, andre.przywara, Stefano Stabellini, shameerali.kolothum.thodi, xen-devel [-- Attachment #1: Type: TEXT/PLAIN, Size: 3501 bytes --] On Wed, 26 Sep 2018, Julien Grall wrote: > Hi Stefano, > > On 09/25/2018 09:45 PM, Stefano Stabellini wrote: > > On Tue, 4 Sep 2018, Andrew Cooper wrote: > > > On 04/09/18 20:35, Julien Grall wrote: > > > > Hi, > > > > > > > > On 09/04/2018 08:21 PM, Julien Grall wrote: > > > > > A follow-up patch will require to know the number of vCPUs when > > > > > initializating the vGICv3 domain structure. However this information > > > > > is > > > > > not available at domain creation. This is only known once > > > > > XEN_DOMCTL_max_vpus is called for that domain. > > > > > > > > > > In order to get the max vCPUs around, delay the domain part of the > > > > > vGIC > > > > > v3 initialization until the first vCPU of the domain is initialized. > > > > > > > > > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > > > > > > > > > --- > > > > > > > > > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > > > > > > > > > This is nasty but I can't find a better way for Xen 4.11 and older. > > > > > This > > > > > is not necessary for unstable as the number of vCPUs is known at > > > > > domain > > > > > creation. > > > > > > > > > > Andrew, I have CCed you to know whether you have a better idea where > > > > > to > > > > > place this call on Xen 4.11 and older. > > > > > > > > I just noticed that d->max_vcpus is initialized after > > > > arch_domain_create. So without this patch on Xen 4.12, it will not work. > > > > > > > > This is getting nastier because arch_domain_init is the one initialize > > > > the value returned by dom0_max_vcpus. So I am not entirely sure what > > > > to do here. > > > > > > The positioning after arch_domain_create() is unfortunate, but I > > > couldn’t manage better with ARM's current behaviour and Jan's insistence > > > that the allocation of d->vcpu was common. I'd prefer if the dependency > > > could be broken and the allocation moved earlier. > > > > > > One option might be to have an arch_check_domainconfig() (or similar?) > > > which is called very early on and can sanity check the values, including > > > cross-checking the vgic and max_vcpus settings? It could even be > > > responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct > > > real value. > > > > > > As for your patch here, its a gross hack, but its probably the best > > > which can be done. > > > > *Sighs* > > If that is what we have to do, it is as ugly as hell, but that is what > > we'll do. > > This is the best we can do with the current code base. I think it would be > worth reworking the code to make it nicer. I will add it in my TODO list. > > > > > My only suggestion to marginally improve it would be instead of: > > > > > + if ( v->vcpu_id == 0 ) > > > + { > > > + rc = vgic_v3_real_domain_init(d); > > > + if ( rc ) > > > + return rc; > > > + } > > > > to check on d->arch.vgic.rdist_regions instead: > > > > if ( d->arch.vgic.rdist_regions == NULL ) > > { > > // initialize domain > > I would prefer to keep v->vcpu_id == 0 just in case we end up to re-order the > allocation in the future. I was suggesting to check on (rdist_regions == NULL) exactly for potential re-ordering, in case in the future we end up calling vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before before vcpu_init(vcpu0). Ideally we would like a way to check that vgic_v3_real_domain_init has been called before and I thought rdist_regions == NULL could do just that... [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-27 23:11 ` Stefano Stabellini @ 2018-09-28 20:35 ` Julien Grall 2018-09-28 23:38 ` Andrew Cooper 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-28 20:35 UTC (permalink / raw) To: Stefano Stabellini Cc: Andrew Cooper, andre.przywara, shameerali.kolothum.thodi, xen-devel On 09/28/2018 12:11 AM, Stefano Stabellini wrote: > On Wed, 26 Sep 2018, Julien Grall wrote: >> Hi Stefano, >> >> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>> On 04/09/18 20:35, Julien Grall wrote: >>>>> Hi, >>>>> >>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>> initializating the vGICv3 domain structure. However this information >>>>>> is >>>>>> not available at domain creation. This is only known once >>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>> >>>>>> In order to get the max vCPUs around, delay the domain part of the >>>>>> vGIC >>>>>> v3 initialization until the first vCPU of the domain is initialized. >>>>>> >>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>> >>>>>> --- >>>>>> >>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>> >>>>>> This is nasty but I can't find a better way for Xen 4.11 and older. >>>>>> This >>>>>> is not necessary for unstable as the number of vCPUs is known at >>>>>> domain >>>>>> creation. >>>>>> >>>>>> Andrew, I have CCed you to know whether you have a better idea where >>>>>> to >>>>>> place this call on Xen 4.11 and older. >>>>> >>>>> I just noticed that d->max_vcpus is initialized after >>>>> arch_domain_create. So without this patch on Xen 4.12, it will not work. >>>>> >>>>> This is getting nastier because arch_domain_init is the one initialize >>>>> the value returned by dom0_max_vcpus. So I am not entirely sure what >>>>> to do here. >>>> >>>> The positioning after arch_domain_create() is unfortunate, but I >>>> couldn’t manage better with ARM's current behaviour and Jan's insistence >>>> that the allocation of d->vcpu was common. I'd prefer if the dependency >>>> could be broken and the allocation moved earlier. >>>> >>>> One option might be to have an arch_check_domainconfig() (or similar?) >>>> which is called very early on and can sanity check the values, including >>>> cross-checking the vgic and max_vcpus settings? It could even be >>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct >>>> real value. >>>> >>>> As for your patch here, its a gross hack, but its probably the best >>>> which can be done. >>> >>> *Sighs* >>> If that is what we have to do, it is as ugly as hell, but that is what >>> we'll do. >> >> This is the best we can do with the current code base. I think it would be >> worth reworking the code to make it nicer. I will add it in my TODO list. >> >>> >>> My only suggestion to marginally improve it would be instead of: >>> >>>> + if ( v->vcpu_id == 0 ) >>>> + { >>>> + rc = vgic_v3_real_domain_init(d); >>>> + if ( rc ) >>>> + return rc; >>>> + } >>> >>> to check on d->arch.vgic.rdist_regions instead: >>> >>> if ( d->arch.vgic.rdist_regions == NULL ) >>> { >>> // initialize domain >> >> I would prefer to keep v->vcpu_id == 0 just in case we end up to re-order the >> allocation in the future. > > I was suggesting to check on (rdist_regions == NULL) exactly for > potential re-ordering, in case in the future we end up calling > vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before > before vcpu_init(vcpu0). Ideally we would like a way to check that > vgic_v3_real_domain_init has been called before and I thought > rdist_regions == NULL could do just that... What I meant by re-ordering is we manage to allocate the re-distributors before the vCPUs are created but still need vgic_v3_real_domain_init for other purpose. But vCPU initialization is potentially other issue. Anyway. both way have drawbacks. Yet I still prefer checking on the vCPU. It less likely vCPU0 will not be the first one initialized. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-28 20:35 ` Julien Grall @ 2018-09-28 23:38 ` Andrew Cooper 2018-09-28 23:45 ` Stefano Stabellini 0 siblings, 1 reply; 24+ messages in thread From: Andrew Cooper @ 2018-09-28 23:38 UTC (permalink / raw) To: Julien Grall, Stefano Stabellini Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara On 28/09/18 21:35, Julien Grall wrote: > > > On 09/28/2018 12:11 AM, Stefano Stabellini wrote: >> On Wed, 26 Sep 2018, Julien Grall wrote: >>> Hi Stefano, >>> >>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>>> On 04/09/18 20:35, Julien Grall wrote: >>>>>> Hi, >>>>>> >>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>>> initializating the vGICv3 domain structure. However this >>>>>>> information >>>>>>> is >>>>>>> not available at domain creation. This is only known once >>>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>>> >>>>>>> In order to get the max vCPUs around, delay the domain part of the >>>>>>> vGIC >>>>>>> v3 initialization until the first vCPU of the domain is >>>>>>> initialized. >>>>>>> >>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>> >>>>>>> --- >>>>>>> >>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>>> >>>>>>> This is nasty but I can't find a better way for Xen 4.11 and older. >>>>>>> This >>>>>>> is not necessary for unstable as the number of vCPUs is known at >>>>>>> domain >>>>>>> creation. >>>>>>> >>>>>>> Andrew, I have CCed you to know whether you have a better idea >>>>>>> where >>>>>>> to >>>>>>> place this call on Xen 4.11 and older. >>>>>> >>>>>> I just noticed that d->max_vcpus is initialized after >>>>>> arch_domain_create. So without this patch on Xen 4.12, it will >>>>>> not work. >>>>>> >>>>>> This is getting nastier because arch_domain_init is the one >>>>>> initialize >>>>>> the value returned by dom0_max_vcpus. So I am not entirely sure what >>>>>> to do here. >>>>> >>>>> The positioning after arch_domain_create() is unfortunate, but I >>>>> couldn’t manage better with ARM's current behaviour and Jan's >>>>> insistence >>>>> that the allocation of d->vcpu was common. I'd prefer if the >>>>> dependency >>>>> could be broken and the allocation moved earlier. >>>>> >>>>> One option might be to have an arch_check_domainconfig() (or >>>>> similar?) >>>>> which is called very early on and can sanity check the values, >>>>> including >>>>> cross-checking the vgic and max_vcpus settings? It could even be >>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the >>>>> correct >>>>> real value. >>>>> >>>>> As for your patch here, its a gross hack, but its probably the best >>>>> which can be done. >>>> >>>> *Sighs* >>>> If that is what we have to do, it is as ugly as hell, but that is what >>>> we'll do. >>> >>> This is the best we can do with the current code base. I think it >>> would be >>> worth reworking the code to make it nicer. I will add it in my TODO >>> list. >>> >>>> >>>> My only suggestion to marginally improve it would be instead of: >>>> >>>>> + if ( v->vcpu_id == 0 ) >>>>> + { >>>>> + rc = vgic_v3_real_domain_init(d); >>>>> + if ( rc ) >>>>> + return rc; >>>>> + } >>>> >>>> to check on d->arch.vgic.rdist_regions instead: >>>> >>>> if ( d->arch.vgic.rdist_regions == NULL ) >>>> { >>>> // initialize domain >>> >>> I would prefer to keep v->vcpu_id == 0 just in case we end up to >>> re-order the >>> allocation in the future. >> >> I was suggesting to check on (rdist_regions == NULL) exactly for >> potential re-ordering, in case in the future we end up calling >> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before >> before vcpu_init(vcpu0). Ideally we would like a way to check that >> vgic_v3_real_domain_init has been called before and I thought >> rdist_regions == NULL could do just that... > > What I meant by re-ordering is we manage to allocate the > re-distributors before the vCPUs are created but still need > vgic_v3_real_domain_init for other purpose. > > But vCPU initialization is potentially other issue. > > Anyway. both way have drawbacks. Yet I still prefer checking on the > vCPU. It less likely vCPU0 will not be the first one initialized. With the exception of the idle domain, all vcpus are strictly allocated in packed ascending order. Loads of other stuff will break if that changed, so I wouldn't worry about it. Furthermore, there is no obvious reason for this behaviour to ever change. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-28 23:38 ` Andrew Cooper @ 2018-09-28 23:45 ` Stefano Stabellini 2018-09-28 23:48 ` Andrew Cooper 0 siblings, 1 reply; 24+ messages in thread From: Stefano Stabellini @ 2018-09-28 23:45 UTC (permalink / raw) To: Andrew Cooper Cc: xen-devel, Julien Grall, Stefano Stabellini, shameerali.kolothum.thodi, andre.przywara [-- Attachment #1: Type: TEXT/PLAIN, Size: 4769 bytes --] On Sat, 29 Sep 2018, Andrew Cooper wrote: > On 28/09/18 21:35, Julien Grall wrote: > > > > > > On 09/28/2018 12:11 AM, Stefano Stabellini wrote: > >> On Wed, 26 Sep 2018, Julien Grall wrote: > >>> Hi Stefano, > >>> > >>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: > >>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: > >>>>> On 04/09/18 20:35, Julien Grall wrote: > >>>>>> Hi, > >>>>>> > >>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: > >>>>>>> A follow-up patch will require to know the number of vCPUs when > >>>>>>> initializating the vGICv3 domain structure. However this > >>>>>>> information > >>>>>>> is > >>>>>>> not available at domain creation. This is only known once > >>>>>>> XEN_DOMCTL_max_vpus is called for that domain. > >>>>>>> > >>>>>>> In order to get the max vCPUs around, delay the domain part of the > >>>>>>> vGIC > >>>>>>> v3 initialization until the first vCPU of the domain is > >>>>>>> initialized. > >>>>>>> > >>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> > >>>>>>> > >>>>>>> --- > >>>>>>> > >>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> > >>>>>>> > >>>>>>> This is nasty but I can't find a better way for Xen 4.11 and older. > >>>>>>> This > >>>>>>> is not necessary for unstable as the number of vCPUs is known at > >>>>>>> domain > >>>>>>> creation. > >>>>>>> > >>>>>>> Andrew, I have CCed you to know whether you have a better idea > >>>>>>> where > >>>>>>> to > >>>>>>> place this call on Xen 4.11 and older. > >>>>>> > >>>>>> I just noticed that d->max_vcpus is initialized after > >>>>>> arch_domain_create. So without this patch on Xen 4.12, it will > >>>>>> not work. > >>>>>> > >>>>>> This is getting nastier because arch_domain_init is the one > >>>>>> initialize > >>>>>> the value returned by dom0_max_vcpus. So I am not entirely sure what > >>>>>> to do here. > >>>>> > >>>>> The positioning after arch_domain_create() is unfortunate, but I > >>>>> couldn’t manage better with ARM's current behaviour and Jan's > >>>>> insistence > >>>>> that the allocation of d->vcpu was common. I'd prefer if the > >>>>> dependency > >>>>> could be broken and the allocation moved earlier. > >>>>> > >>>>> One option might be to have an arch_check_domainconfig() (or > >>>>> similar?) > >>>>> which is called very early on and can sanity check the values, > >>>>> including > >>>>> cross-checking the vgic and max_vcpus settings? It could even be > >>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the > >>>>> correct > >>>>> real value. > >>>>> > >>>>> As for your patch here, its a gross hack, but its probably the best > >>>>> which can be done. > >>>> > >>>> *Sighs* > >>>> If that is what we have to do, it is as ugly as hell, but that is what > >>>> we'll do. > >>> > >>> This is the best we can do with the current code base. I think it > >>> would be > >>> worth reworking the code to make it nicer. I will add it in my TODO > >>> list. > >>> > >>>> > >>>> My only suggestion to marginally improve it would be instead of: > >>>> > >>>>> + if ( v->vcpu_id == 0 ) > >>>>> + { > >>>>> + rc = vgic_v3_real_domain_init(d); > >>>>> + if ( rc ) > >>>>> + return rc; > >>>>> + } > >>>> > >>>> to check on d->arch.vgic.rdist_regions instead: > >>>> > >>>> if ( d->arch.vgic.rdist_regions == NULL ) > >>>> { > >>>> // initialize domain > >>> > >>> I would prefer to keep v->vcpu_id == 0 just in case we end up to > >>> re-order the > >>> allocation in the future. > >> > >> I was suggesting to check on (rdist_regions == NULL) exactly for > >> potential re-ordering, in case in the future we end up calling > >> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before > >> before vcpu_init(vcpu0). Ideally we would like a way to check that > >> vgic_v3_real_domain_init has been called before and I thought > >> rdist_regions == NULL could do just that... > > > > What I meant by re-ordering is we manage to allocate the > > re-distributors before the vCPUs are created but still need > > vgic_v3_real_domain_init for other purpose. > > > > But vCPU initialization is potentially other issue. > > > > Anyway. both way have drawbacks. Yet I still prefer checking on the > > vCPU. It less likely vCPU0 will not be the first one initialized. > > With the exception of the idle domain, all vcpus are strictly allocated > in packed ascending order. Loads of other stuff will break if that > changed, so I wouldn't worry about it. > > Furthermore, there is no obvious reason for this behaviour to ever change. OK, let's go with Julien's patch. We need a new tag for this, something like: Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org> :-) [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-28 23:45 ` Stefano Stabellini @ 2018-09-28 23:48 ` Andrew Cooper 2018-10-01 9:43 ` Julien Grall 0 siblings, 1 reply; 24+ messages in thread From: Andrew Cooper @ 2018-09-28 23:48 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Julien Grall, shameerali.kolothum.thodi, andre.przywara On 29/09/18 00:45, Stefano Stabellini wrote: > On Sat, 29 Sep 2018, Andrew Cooper wrote: >> On 28/09/18 21:35, Julien Grall wrote: >>> >>> On 09/28/2018 12:11 AM, Stefano Stabellini wrote: >>>> On Wed, 26 Sep 2018, Julien Grall wrote: >>>>> Hi Stefano, >>>>> >>>>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>>>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>>>>> On 04/09/18 20:35, Julien Grall wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>>>>> initializating the vGICv3 domain structure. However this >>>>>>>>> information >>>>>>>>> is >>>>>>>>> not available at domain creation. This is only known once >>>>>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>>>>> >>>>>>>>> In order to get the max vCPUs around, delay the domain part of the >>>>>>>>> vGIC >>>>>>>>> v3 initialization until the first vCPU of the domain is >>>>>>>>> initialized. >>>>>>>>> >>>>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>>>> >>>>>>>>> --- >>>>>>>>> >>>>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>>>>> >>>>>>>>> This is nasty but I can't find a better way for Xen 4.11 and older. >>>>>>>>> This >>>>>>>>> is not necessary for unstable as the number of vCPUs is known at >>>>>>>>> domain >>>>>>>>> creation. >>>>>>>>> >>>>>>>>> Andrew, I have CCed you to know whether you have a better idea >>>>>>>>> where >>>>>>>>> to >>>>>>>>> place this call on Xen 4.11 and older. >>>>>>>> I just noticed that d->max_vcpus is initialized after >>>>>>>> arch_domain_create. So without this patch on Xen 4.12, it will >>>>>>>> not work. >>>>>>>> >>>>>>>> This is getting nastier because arch_domain_init is the one >>>>>>>> initialize >>>>>>>> the value returned by dom0_max_vcpus. So I am not entirely sure what >>>>>>>> to do here. >>>>>>> The positioning after arch_domain_create() is unfortunate, but I >>>>>>> couldn’t manage better with ARM's current behaviour and Jan's >>>>>>> insistence >>>>>>> that the allocation of d->vcpu was common. I'd prefer if the >>>>>>> dependency >>>>>>> could be broken and the allocation moved earlier. >>>>>>> >>>>>>> One option might be to have an arch_check_domainconfig() (or >>>>>>> similar?) >>>>>>> which is called very early on and can sanity check the values, >>>>>>> including >>>>>>> cross-checking the vgic and max_vcpus settings? It could even be >>>>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the >>>>>>> correct >>>>>>> real value. >>>>>>> >>>>>>> As for your patch here, its a gross hack, but its probably the best >>>>>>> which can be done. >>>>>> *Sighs* >>>>>> If that is what we have to do, it is as ugly as hell, but that is what >>>>>> we'll do. >>>>> This is the best we can do with the current code base. I think it >>>>> would be >>>>> worth reworking the code to make it nicer. I will add it in my TODO >>>>> list. >>>>> >>>>>> My only suggestion to marginally improve it would be instead of: >>>>>> >>>>>>> + if ( v->vcpu_id == 0 ) >>>>>>> + { >>>>>>> + rc = vgic_v3_real_domain_init(d); >>>>>>> + if ( rc ) >>>>>>> + return rc; >>>>>>> + } >>>>>> to check on d->arch.vgic.rdist_regions instead: >>>>>> >>>>>> if ( d->arch.vgic.rdist_regions == NULL ) >>>>>> { >>>>>> // initialize domain >>>>> I would prefer to keep v->vcpu_id == 0 just in case we end up to >>>>> re-order the >>>>> allocation in the future. >>>> I was suggesting to check on (rdist_regions == NULL) exactly for >>>> potential re-ordering, in case in the future we end up calling >>>> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before >>>> before vcpu_init(vcpu0). Ideally we would like a way to check that >>>> vgic_v3_real_domain_init has been called before and I thought >>>> rdist_regions == NULL could do just that... >>> What I meant by re-ordering is we manage to allocate the >>> re-distributors before the vCPUs are created but still need >>> vgic_v3_real_domain_init for other purpose. >>> >>> But vCPU initialization is potentially other issue. >>> >>> Anyway. both way have drawbacks. Yet I still prefer checking on the >>> vCPU. It less likely vCPU0 will not be the first one initialized. >> With the exception of the idle domain, all vcpus are strictly allocated >> in packed ascending order. Loads of other stuff will break if that >> changed, so I wouldn't worry about it. >> >> Furthermore, there is no obvious reason for this behaviour to ever change. > OK, let's go with Julien's patch. We need a new tag for this, something > like: > > Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org> Do bear in mind that this patch is only for 4.11 and earlier. I've already fixed staging (i.e. 4.12) when it comes to knowing d->max_vcpus :) ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-09-28 23:48 ` Andrew Cooper @ 2018-10-01 9:43 ` Julien Grall 2018-10-01 9:53 ` Andrew Cooper 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-10-01 9:43 UTC (permalink / raw) To: Andrew Cooper, Stefano Stabellini Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara Hi, On 09/29/2018 12:48 AM, Andrew Cooper wrote: > On 29/09/18 00:45, Stefano Stabellini wrote: >> On Sat, 29 Sep 2018, Andrew Cooper wrote: >>> On 28/09/18 21:35, Julien Grall wrote: >>>> >>>> On 09/28/2018 12:11 AM, Stefano Stabellini wrote: >>>>> On Wed, 26 Sep 2018, Julien Grall wrote: >>>>>> Hi Stefano, >>>>>> >>>>>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>>>>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>>>>>> On 04/09/18 20:35, Julien Grall wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>>>>>> initializating the vGICv3 domain structure. However this >>>>>>>>>> information >>>>>>>>>> is >>>>>>>>>> not available at domain creation. This is only known once >>>>>>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>>>>>> >>>>>>>>>> In order to get the max vCPUs around, delay the domain part of the >>>>>>>>>> vGIC >>>>>>>>>> v3 initialization until the first vCPU of the domain is >>>>>>>>>> initialized. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>>>>> >>>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>>>>>> >>>>>>>>>> This is nasty but I can't find a better way for Xen 4.11 and older. >>>>>>>>>> This >>>>>>>>>> is not necessary for unstable as the number of vCPUs is known at >>>>>>>>>> domain >>>>>>>>>> creation. >>>>>>>>>> >>>>>>>>>> Andrew, I have CCed you to know whether you have a better idea >>>>>>>>>> where >>>>>>>>>> to >>>>>>>>>> place this call on Xen 4.11 and older. >>>>>>>>> I just noticed that d->max_vcpus is initialized after >>>>>>>>> arch_domain_create. So without this patch on Xen 4.12, it will >>>>>>>>> not work. >>>>>>>>> >>>>>>>>> This is getting nastier because arch_domain_init is the one >>>>>>>>> initialize >>>>>>>>> the value returned by dom0_max_vcpus. So I am not entirely sure what >>>>>>>>> to do here. >>>>>>>> The positioning after arch_domain_create() is unfortunate, but I >>>>>>>> couldn’t manage better with ARM's current behaviour and Jan's >>>>>>>> insistence >>>>>>>> that the allocation of d->vcpu was common. I'd prefer if the >>>>>>>> dependency >>>>>>>> could be broken and the allocation moved earlier. >>>>>>>> >>>>>>>> One option might be to have an arch_check_domainconfig() (or >>>>>>>> similar?) >>>>>>>> which is called very early on and can sanity check the values, >>>>>>>> including >>>>>>>> cross-checking the vgic and max_vcpus settings? It could even be >>>>>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the >>>>>>>> correct >>>>>>>> real value. >>>>>>>> >>>>>>>> As for your patch here, its a gross hack, but its probably the best >>>>>>>> which can be done. >>>>>>> *Sighs* >>>>>>> If that is what we have to do, it is as ugly as hell, but that is what >>>>>>> we'll do. >>>>>> This is the best we can do with the current code base. I think it >>>>>> would be >>>>>> worth reworking the code to make it nicer. I will add it in my TODO >>>>>> list. >>>>>> >>>>>>> My only suggestion to marginally improve it would be instead of: >>>>>>> >>>>>>>> + if ( v->vcpu_id == 0 ) >>>>>>>> + { >>>>>>>> + rc = vgic_v3_real_domain_init(d); >>>>>>>> + if ( rc ) >>>>>>>> + return rc; >>>>>>>> + } >>>>>>> to check on d->arch.vgic.rdist_regions instead: >>>>>>> >>>>>>> if ( d->arch.vgic.rdist_regions == NULL ) >>>>>>> { >>>>>>> // initialize domain >>>>>> I would prefer to keep v->vcpu_id == 0 just in case we end up to >>>>>> re-order the >>>>>> allocation in the future. >>>>> I was suggesting to check on (rdist_regions == NULL) exactly for >>>>> potential re-ordering, in case in the future we end up calling >>>>> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before >>>>> before vcpu_init(vcpu0). Ideally we would like a way to check that >>>>> vgic_v3_real_domain_init has been called before and I thought >>>>> rdist_regions == NULL could do just that... >>>> What I meant by re-ordering is we manage to allocate the >>>> re-distributors before the vCPUs are created but still need >>>> vgic_v3_real_domain_init for other purpose. >>>> >>>> But vCPU initialization is potentially other issue. >>>> >>>> Anyway. both way have drawbacks. Yet I still prefer checking on the >>>> vCPU. It less likely vCPU0 will not be the first one initialized. >>> With the exception of the idle domain, all vcpus are strictly allocated >>> in packed ascending order. Loads of other stuff will break if that >>> changed, so I wouldn't worry about it. >>> >>> Furthermore, there is no obvious reason for this behaviour to ever change. >> OK, let's go with Julien's patch. We need a new tag for this, something >> like: >> >> Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org> > > Do bear in mind that this patch is only for 4.11 and earlier. I've > already fixed staging (i.e. 4.12) when it comes to knowing d->max_vcpus :) I thought we agreed that patch is necessary for 4.12 as d->max_vcpus is initialized after arch_domain_init? I am not planning to do the rework in short term. Did you do more work on around domain_create recently? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-10-01 9:43 ` Julien Grall @ 2018-10-01 9:53 ` Andrew Cooper 2018-10-01 11:31 ` Julien Grall 0 siblings, 1 reply; 24+ messages in thread From: Andrew Cooper @ 2018-10-01 9:53 UTC (permalink / raw) To: Julien Grall, Stefano Stabellini Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara On 01/10/18 10:43, Julien Grall wrote: > Hi, > > On 09/29/2018 12:48 AM, Andrew Cooper wrote: >> On 29/09/18 00:45, Stefano Stabellini wrote: >>> On Sat, 29 Sep 2018, Andrew Cooper wrote: >>>> On 28/09/18 21:35, Julien Grall wrote: >>>>> >>>>> On 09/28/2018 12:11 AM, Stefano Stabellini wrote: >>>>>> On Wed, 26 Sep 2018, Julien Grall wrote: >>>>>>> Hi Stefano, >>>>>>> >>>>>>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>>>>>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>>>>>>> On 04/09/18 20:35, Julien Grall wrote: >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>>>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>>>>>>> initializating the vGICv3 domain structure. However this >>>>>>>>>>> information >>>>>>>>>>> is >>>>>>>>>>> not available at domain creation. This is only known once >>>>>>>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>>>>>>> >>>>>>>>>>> In order to get the max vCPUs around, delay the domain part >>>>>>>>>>> of the >>>>>>>>>>> vGIC >>>>>>>>>>> v3 initialization until the first vCPU of the domain is >>>>>>>>>>> initialized. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>>>>>> >>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>>>>>>> >>>>>>>>>>> This is nasty but I can't find a better way for Xen 4.11 and >>>>>>>>>>> older. >>>>>>>>>>> This >>>>>>>>>>> is not necessary for unstable as the number of vCPUs is >>>>>>>>>>> known at >>>>>>>>>>> domain >>>>>>>>>>> creation. >>>>>>>>>>> >>>>>>>>>>> Andrew, I have CCed you to know whether you have a better idea >>>>>>>>>>> where >>>>>>>>>>> to >>>>>>>>>>> place this call on Xen 4.11 and older. >>>>>>>>>> I just noticed that d->max_vcpus is initialized after >>>>>>>>>> arch_domain_create. So without this patch on Xen 4.12, it will >>>>>>>>>> not work. >>>>>>>>>> >>>>>>>>>> This is getting nastier because arch_domain_init is the one >>>>>>>>>> initialize >>>>>>>>>> the value returned by dom0_max_vcpus. So I am not entirely >>>>>>>>>> sure what >>>>>>>>>> to do here. >>>>>>>>> The positioning after arch_domain_create() is unfortunate, but I >>>>>>>>> couldn’t manage better with ARM's current behaviour and Jan's >>>>>>>>> insistence >>>>>>>>> that the allocation of d->vcpu was common. I'd prefer if the >>>>>>>>> dependency >>>>>>>>> could be broken and the allocation moved earlier. >>>>>>>>> >>>>>>>>> One option might be to have an arch_check_domainconfig() (or >>>>>>>>> similar?) >>>>>>>>> which is called very early on and can sanity check the values, >>>>>>>>> including >>>>>>>>> cross-checking the vgic and max_vcpus settings? It could even be >>>>>>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the >>>>>>>>> correct >>>>>>>>> real value. >>>>>>>>> >>>>>>>>> As for your patch here, its a gross hack, but its probably the >>>>>>>>> best >>>>>>>>> which can be done. >>>>>>>> *Sighs* >>>>>>>> If that is what we have to do, it is as ugly as hell, but that >>>>>>>> is what >>>>>>>> we'll do. >>>>>>> This is the best we can do with the current code base. I think it >>>>>>> would be >>>>>>> worth reworking the code to make it nicer. I will add it in my TODO >>>>>>> list. >>>>>>> >>>>>>>> My only suggestion to marginally improve it would be instead of: >>>>>>>> >>>>>>>>> + if ( v->vcpu_id == 0 ) >>>>>>>>> + { >>>>>>>>> + rc = vgic_v3_real_domain_init(d); >>>>>>>>> + if ( rc ) >>>>>>>>> + return rc; >>>>>>>>> + } >>>>>>>> to check on d->arch.vgic.rdist_regions instead: >>>>>>>> >>>>>>>> if ( d->arch.vgic.rdist_regions == NULL ) >>>>>>>> { >>>>>>>> // initialize domain >>>>>>> I would prefer to keep v->vcpu_id == 0 just in case we end up to >>>>>>> re-order the >>>>>>> allocation in the future. >>>>>> I was suggesting to check on (rdist_regions == NULL) exactly for >>>>>> potential re-ordering, in case in the future we end up calling >>>>>> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done >>>>>> before >>>>>> before vcpu_init(vcpu0). Ideally we would like a way to check that >>>>>> vgic_v3_real_domain_init has been called before and I thought >>>>>> rdist_regions == NULL could do just that... >>>>> What I meant by re-ordering is we manage to allocate the >>>>> re-distributors before the vCPUs are created but still need >>>>> vgic_v3_real_domain_init for other purpose. >>>>> >>>>> But vCPU initialization is potentially other issue. >>>>> >>>>> Anyway. both way have drawbacks. Yet I still prefer checking on the >>>>> vCPU. It less likely vCPU0 will not be the first one initialized. >>>> With the exception of the idle domain, all vcpus are strictly >>>> allocated >>>> in packed ascending order. Loads of other stuff will break if that >>>> changed, so I wouldn't worry about it. >>>> >>>> Furthermore, there is no obvious reason for this behaviour to ever >>>> change. >>> OK, let's go with Julien's patch. We need a new tag for this, something >>> like: >>> >>> Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org> >> >> Do bear in mind that this patch is only for 4.11 and earlier. I've >> already fixed staging (i.e. 4.12) when it comes to knowing >> d->max_vcpus :) > I thought we agreed that patch is necessary for 4.12 as d->max_vcpus > is initialized after arch_domain_init? Oh right. > I am not planning to do the rework in short term. Did you do more work > on around domain_create recently? There are multiple related patch series out on xen-devel atm, but I expect I need to spin a new version of each of them. I'll see if I have some time to put towards it. Are you happy in principle with the arch_check_domainconfig() plan? ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information 2018-10-01 9:53 ` Andrew Cooper @ 2018-10-01 11:31 ` Julien Grall 0 siblings, 0 replies; 24+ messages in thread From: Julien Grall @ 2018-10-01 11:31 UTC (permalink / raw) To: Andrew Cooper, Stefano Stabellini Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara Hi Andrew, On 10/01/2018 10:53 AM, Andrew Cooper wrote: > On 01/10/18 10:43, Julien Grall wrote: >> Hi, >> >> On 09/29/2018 12:48 AM, Andrew Cooper wrote: >>> On 29/09/18 00:45, Stefano Stabellini wrote: >>>> On Sat, 29 Sep 2018, Andrew Cooper wrote: >>>>> On 28/09/18 21:35, Julien Grall wrote: >>>>>> >>>>>> On 09/28/2018 12:11 AM, Stefano Stabellini wrote: >>>>>>> On Wed, 26 Sep 2018, Julien Grall wrote: >>>>>>>> Hi Stefano, >>>>>>>> >>>>>>>> On 09/25/2018 09:45 PM, Stefano Stabellini wrote: >>>>>>>>> On Tue, 4 Sep 2018, Andrew Cooper wrote: >>>>>>>>>> On 04/09/18 20:35, Julien Grall wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> On 09/04/2018 08:21 PM, Julien Grall wrote: >>>>>>>>>>>> A follow-up patch will require to know the number of vCPUs when >>>>>>>>>>>> initializating the vGICv3 domain structure. However this >>>>>>>>>>>> information >>>>>>>>>>>> is >>>>>>>>>>>> not available at domain creation. This is only known once >>>>>>>>>>>> XEN_DOMCTL_max_vpus is called for that domain. >>>>>>>>>>>> >>>>>>>>>>>> In order to get the max vCPUs around, delay the domain part >>>>>>>>>>>> of the >>>>>>>>>>>> vGIC >>>>>>>>>>>> v3 initialization until the first vCPU of the domain is >>>>>>>>>>>> initialized. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>>>>>>> >>>>>>>>>>>> --- >>>>>>>>>>>> >>>>>>>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>>>>>>>> >>>>>>>>>>>> This is nasty but I can't find a better way for Xen 4.11 and >>>>>>>>>>>> older. >>>>>>>>>>>> This >>>>>>>>>>>> is not necessary for unstable as the number of vCPUs is >>>>>>>>>>>> known at >>>>>>>>>>>> domain >>>>>>>>>>>> creation. >>>>>>>>>>>> >>>>>>>>>>>> Andrew, I have CCed you to know whether you have a better idea >>>>>>>>>>>> where >>>>>>>>>>>> to >>>>>>>>>>>> place this call on Xen 4.11 and older. >>>>>>>>>>> I just noticed that d->max_vcpus is initialized after >>>>>>>>>>> arch_domain_create. So without this patch on Xen 4.12, it will >>>>>>>>>>> not work. >>>>>>>>>>> >>>>>>>>>>> This is getting nastier because arch_domain_init is the one >>>>>>>>>>> initialize >>>>>>>>>>> the value returned by dom0_max_vcpus. So I am not entirely >>>>>>>>>>> sure what >>>>>>>>>>> to do here. >>>>>>>>>> The positioning after arch_domain_create() is unfortunate, but I >>>>>>>>>> couldn’t manage better with ARM's current behaviour and Jan's >>>>>>>>>> insistence >>>>>>>>>> that the allocation of d->vcpu was common. I'd prefer if the >>>>>>>>>> dependency >>>>>>>>>> could be broken and the allocation moved earlier. >>>>>>>>>> >>>>>>>>>> One option might be to have an arch_check_domainconfig() (or >>>>>>>>>> similar?) >>>>>>>>>> which is called very early on and can sanity check the values, >>>>>>>>>> including >>>>>>>>>> cross-checking the vgic and max_vcpus settings? It could even be >>>>>>>>>> responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the >>>>>>>>>> correct >>>>>>>>>> real value. >>>>>>>>>> >>>>>>>>>> As for your patch here, its a gross hack, but its probably the >>>>>>>>>> best >>>>>>>>>> which can be done. >>>>>>>>> *Sighs* >>>>>>>>> If that is what we have to do, it is as ugly as hell, but that >>>>>>>>> is what >>>>>>>>> we'll do. >>>>>>>> This is the best we can do with the current code base. I think it >>>>>>>> would be >>>>>>>> worth reworking the code to make it nicer. I will add it in my TODO >>>>>>>> list. >>>>>>>> >>>>>>>>> My only suggestion to marginally improve it would be instead of: >>>>>>>>> >>>>>>>>>> + if ( v->vcpu_id == 0 ) >>>>>>>>>> + { >>>>>>>>>> + rc = vgic_v3_real_domain_init(d); >>>>>>>>>> + if ( rc ) >>>>>>>>>> + return rc; >>>>>>>>>> + } >>>>>>>>> to check on d->arch.vgic.rdist_regions instead: >>>>>>>>> >>>>>>>>> if ( d->arch.vgic.rdist_regions == NULL ) >>>>>>>>> { >>>>>>>>> // initialize domain >>>>>>>> I would prefer to keep v->vcpu_id == 0 just in case we end up to >>>>>>>> re-order the >>>>>>>> allocation in the future. >>>>>>> I was suggesting to check on (rdist_regions == NULL) exactly for >>>>>>> potential re-ordering, in case in the future we end up calling >>>>>>> vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done >>>>>>> before >>>>>>> before vcpu_init(vcpu0). Ideally we would like a way to check that >>>>>>> vgic_v3_real_domain_init has been called before and I thought >>>>>>> rdist_regions == NULL could do just that... >>>>>> What I meant by re-ordering is we manage to allocate the >>>>>> re-distributors before the vCPUs are created but still need >>>>>> vgic_v3_real_domain_init for other purpose. >>>>>> >>>>>> But vCPU initialization is potentially other issue. >>>>>> >>>>>> Anyway. both way have drawbacks. Yet I still prefer checking on the >>>>>> vCPU. It less likely vCPU0 will not be the first one initialized. >>>>> With the exception of the idle domain, all vcpus are strictly >>>>> allocated >>>>> in packed ascending order. Loads of other stuff will break if that >>>>> changed, so I wouldn't worry about it. >>>>> >>>>> Furthermore, there is no obvious reason for this behaviour to ever >>>>> change. >>>> OK, let's go with Julien's patch. We need a new tag for this, something >>>> like: >>>> >>>> Acked-but-disliked-by: Stefano Stabellini <sstabellini@kernel.org> >>> >>> Do bear in mind that this patch is only for 4.11 and earlier. I've >>> already fixed staging (i.e. 4.12) when it comes to knowing >>> d->max_vcpus :) >> I thought we agreed that patch is necessary for 4.12 as d->max_vcpus >> is initialized after arch_domain_init? > > Oh right. > >> I am not planning to do the rework in short term. Did you do more work >> on around domain_create recently? > > There are multiple related patch series out on xen-devel atm, but I > expect I need to spin a new version of each of them. I'll see if I have > some time to put towards it. Are you happy in principle with the > arch_check_domainconfig() plan? I am happy in principle. If you don't have time to work on it, I will try to have a look before Xen 4.12. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-04 19:21 [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Julien Grall 2018-09-04 19:21 ` [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall @ 2018-09-04 19:21 ` Julien Grall 2018-09-25 20:38 ` Stefano Stabellini 2018-09-04 19:21 ` [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent Julien Grall 2018-09-06 15:49 ` [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Shameerali Kolothum Thodi 3 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-04 19:21 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, sstabellini, shameerali.kolothum.thodi At the moment, Xen is assuming the hardware domain will have the same number of re-distributor regions as the host. However, as the number of CPUs or the stride (e.g on GICv4) may be different we end up exposing regions which does not contain any re-distributors. When booting, Linux will go through all the re-distributor region to check whether a property (e.g vPLIs) is available accross all the re-distributors. This will result to a data abort on empty regions because there are no underlying re-distributor. So we need to limit the number of regions exposed to the hardware domain. The code reworked to only expose the minimun number of regions required by the hardware domain. It is assumed the regions will be populated starting from the first one. Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/gic-v3.c | 10 ++++++++-- xen/arch/arm/vgic-v3.c | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index b2ed0f8b55..4a984cfb12 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1274,8 +1274,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, * GIC has two memory regions: Distributor + rdist regions * CPU interface and virtual cpu interfaces accessesed as System registers * So cells are created only for Distributor and rdist regions + * The hardware domain may not used all the regions. So only copy + * what is necessary. */ - new_len = new_len * (gicv3.rdist_count + 1); + new_len = new_len * (d->arch.vgic.nr_regions + 1); hw_reg = dt_get_property(gic, "reg", &len); if ( !hw_reg ) @@ -1503,7 +1505,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) /* Add Generic Redistributor */ size = sizeof(struct acpi_madt_generic_redistributor); - for ( i = 0; i < gicv3.rdist_count; i++ ) + /* + * The hardware domain may not used all the regions. So only copy + * what is necessary. + */ + for ( i = 0; i < d->arch.vgic.nr_regions; i++ ) { gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + table_len); gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR; diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index df1bab3a35..9f729862da 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1695,8 +1695,19 @@ static int vgic_v3_real_domain_init(struct domain *d) d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; first_cpu += size / GICV3_GICR_SIZE; + + if ( first_cpu >= d->max_vcpus ) + break; } + /* + * The hardware domain may not used all the re-distributors + * regions (e.g when the number of vCPUs does not match the + * number of pCPUs). Update the number of regions to avoid + * exposing unused region as they will not get emulated. + */ + d->arch.vgic.nr_regions = i + 1; + d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits; } else -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-04 19:21 ` [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall @ 2018-09-25 20:38 ` Stefano Stabellini 2018-09-26 20:36 ` Julien Grall 0 siblings, 1 reply; 24+ messages in thread From: Stefano Stabellini @ 2018-09-25 20:38 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, sstabellini, shameerali.kolothum.thodi, andre.przywara On Tue, 4 Sep 2018, Julien Grall wrote: > At the moment, Xen is assuming the hardware domain will have the same > number of re-distributor regions as the host. However, as the > number of CPUs or the stride (e.g on GICv4) may be different we end up > exposing regions which does not contain any re-distributors. > > When booting, Linux will go through all the re-distributor region to > check whether a property (e.g vPLIs) is available accross all the > re-distributors. This will result to a data abort on empty regions > because there are no underlying re-distributor. > > So we need to limit the number of regions exposed to the hardware > domain. The code reworked to only expose the minimun number of regions > required by the hardware domain. It is assumed the regions will be > populated starting from the first one. I have a question: given that it is possible for a rdist_region to cover more than 1 cpu, could we get into troubles if the last rdist_region of the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write would return 0. This case seems to be handled correctly but I wanted to double check. I think we also need to fix vgic_v3_rdist_count? Today it just returns vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it unfixed? If we do that, we might be able to get rid of the modifications to vgic_v3_real_domain_init. > Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/gic-v3.c | 10 ++++++++-- > xen/arch/arm/vgic-v3.c | 11 +++++++++++ > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index b2ed0f8b55..4a984cfb12 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1274,8 +1274,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, > * GIC has two memory regions: Distributor + rdist regions > * CPU interface and virtual cpu interfaces accessesed as System registers > * So cells are created only for Distributor and rdist regions > + * The hardware domain may not used all the regions. So only copy > + * what is necessary. > */ > - new_len = new_len * (gicv3.rdist_count + 1); > + new_len = new_len * (d->arch.vgic.nr_regions + 1); Do we also need to fix "#redistributor-regions" just above? > hw_reg = dt_get_property(gic, "reg", &len); > if ( !hw_reg ) > @@ -1503,7 +1505,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) > > /* Add Generic Redistributor */ > size = sizeof(struct acpi_madt_generic_redistributor); > - for ( i = 0; i < gicv3.rdist_count; i++ ) > + /* > + * The hardware domain may not used all the regions. So only copy ^ use > + * what is necessary. > + */ > + for ( i = 0; i < d->arch.vgic.nr_regions; i++ ) > { > gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + table_len); > gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR; > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index df1bab3a35..9f729862da 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -1695,8 +1695,19 @@ static int vgic_v3_real_domain_init(struct domain *d) > d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; > > first_cpu += size / GICV3_GICR_SIZE; > + > + if ( first_cpu >= d->max_vcpus ) > + break; This is just a matter of code style and preferences, but I would prefer if the termination condition was at the top as part of the for statement. Of course, it works regardless, so the patch would be OK either way. > } > > + /* > + * The hardware domain may not used all the re-distributors ^ use > + * regions (e.g when the number of vCPUs does not match the > + * number of pCPUs). Update the number of regions to avoid > + * exposing unused region as they will not get emulated. ^ regions > + */ > + d->arch.vgic.nr_regions = i + 1; > d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits; > } > else _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-25 20:38 ` Stefano Stabellini @ 2018-09-26 20:36 ` Julien Grall 2018-09-27 23:34 ` Stefano Stabellini 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-26 20:36 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara Hi Stefano, On 09/25/2018 09:38 PM, Stefano Stabellini wrote: > On Tue, 4 Sep 2018, Julien Grall wrote: >> At the moment, Xen is assuming the hardware domain will have the same >> number of re-distributor regions as the host. However, as the >> number of CPUs or the stride (e.g on GICv4) may be different we end up >> exposing regions which does not contain any re-distributors. >> >> When booting, Linux will go through all the re-distributor region to >> check whether a property (e.g vPLIs) is available accross all the >> re-distributors. This will result to a data abort on empty regions >> because there are no underlying re-distributor. >> >> So we need to limit the number of regions exposed to the hardware >> domain. The code reworked to only expose the minimun number of regions >> required by the hardware domain. It is assumed the regions will be >> populated starting from the first one. > > I have a question: given that it is possible for a rdist_region to cover > more than 1 cpu, could we get into troubles if the last rdist_region of > the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? > get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write > would return 0. > This case seems to be handled correctly but I wanted to > double check. 0 means a data abort will be injected into the guest. However, the guest should never touch that because the last valid re-distributor of the regions will have GICR_TYPER.Last set. So the guest OS will stop looking for more re-distributors in that region. > > > I think we also need to fix vgic_v3_rdist_count? Today it just returns > vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it > unfixed? If we do that, we might be able to get rid of the modifications > to vgic_v3_real_domain_init. We don't want to fix vgic_v3_rdist_count. The helper returns the maximum re-distributors regions. This is used to compute the number of IO handlers and allocating the array storing the regions. I am pretty sure you will say we will waste memory. However, at the moment, we need to know the number of IO handlers much before we know the number of vCPUs. For the array, we would need to go through the regions twice (regions may not be the same size so we can't infer easily the number needed). Overall, the amount of memory used is the same as today (so not really a waste per-se). It might be possible to limit that once we reworked the common code to know the number of vCPUs earlier on (see discussion on patch #1). >> Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/gic-v3.c | 10 ++++++++-- >> xen/arch/arm/vgic-v3.c | 11 +++++++++++ >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c >> index b2ed0f8b55..4a984cfb12 100644 >> --- a/xen/arch/arm/gic-v3.c >> +++ b/xen/arch/arm/gic-v3.c >> @@ -1274,8 +1274,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, >> * GIC has two memory regions: Distributor + rdist regions >> * CPU interface and virtual cpu interfaces accessesed as System registers >> * So cells are created only for Distributor and rdist regions >> + * The hardware domain may not used all the regions. So only copy >> + * what is necessary. >> */ >> - new_len = new_len * (gicv3.rdist_count + 1); >> + new_len = new_len * (d->arch.vgic.nr_regions + 1); > > Do we also need to fix "#redistributor-regions" just above? Hmm I missed that one. Not sure why it didn't show up in my test. >> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c >> index df1bab3a35..9f729862da 100644 >> --- a/xen/arch/arm/vgic-v3.c >> +++ b/xen/arch/arm/vgic-v3.c >> @@ -1695,8 +1695,19 @@ static int vgic_v3_real_domain_init(struct domain *d) >> d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; >> >> first_cpu += size / GICV3_GICR_SIZE; >> + >> + if ( first_cpu >= d->max_vcpus ) >> + break; > > This is just a matter of code style and preferences, but I would prefer > if the termination condition was at the top as part of the for > statement. Of course, it works regardless, so the patch would be > OK either way. I thought about it when writing this patch. This would look like: for ( i = 0; (i < vgic_v3_hw.nr_dist_regions) && (first_cpu < d->max_vcpus); i++ ) This is IHMO more difficult to understand (long condition) and slightly strange because for is not incrementing directly first_cpu. I will stick with the current implementation unless you have a more readable solution. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-26 20:36 ` Julien Grall @ 2018-09-27 23:34 ` Stefano Stabellini 2018-09-28 20:37 ` Julien Grall 0 siblings, 1 reply; 24+ messages in thread From: Stefano Stabellini @ 2018-09-27 23:34 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Stefano Stabellini, shameerali.kolothum.thodi, andre.przywara On Wed, 26 Sep 2018, Julien Grall wrote: > Hi Stefano, > > On 09/25/2018 09:38 PM, Stefano Stabellini wrote: > > On Tue, 4 Sep 2018, Julien Grall wrote: > > > At the moment, Xen is assuming the hardware domain will have the same > > > number of re-distributor regions as the host. However, as the > > > number of CPUs or the stride (e.g on GICv4) may be different we end up > > > exposing regions which does not contain any re-distributors. > > > > > > When booting, Linux will go through all the re-distributor region to > > > check whether a property (e.g vPLIs) is available accross all the > > > re-distributors. This will result to a data abort on empty regions > > > because there are no underlying re-distributor. > > > > > > So we need to limit the number of regions exposed to the hardware > > > domain. The code reworked to only expose the minimun number of regions > > > required by the hardware domain. It is assumed the regions will be > > > populated starting from the first one. > > > > I have a question: given that it is possible for a rdist_region to cover > > more than 1 cpu, could we get into troubles if the last rdist_region of > > the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? > > get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write > > would return 0. > > This case seems to be handled correctly but I wanted to > > double check. > > 0 means a data abort will be injected into the guest. However, the guest > should never touch that because the last valid re-distributor of the regions > will have GICR_TYPER.Last set. > > So the guest OS will stop looking for more re-distributors in that region. OK > > > > > I think we also need to fix vgic_v3_rdist_count? Today it just returns > > vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it > > unfixed? If we do that, we might be able to get rid of the modifications > > to vgic_v3_real_domain_init. > > We don't want to fix vgic_v3_rdist_count. The helper returns the maximum > re-distributors regions. We don't want to or we can't? Because it looks like we would want to fix vgic_v3_rdist_count if we could, right? It is called from domain specific initialization functions, so theoretically it should return domain specific vgic information, not physical information. > This is used to compute the number of IO handlers and > allocating the array storing the regions. > > I am pretty sure you will say we will waste memory. However, at the moment, > we need to know the number of IO handlers much before we know the number of > vCPUs. For the array, we would need to go through the regions twice (regions > may not be the same size so we can't infer easily the number needed). Overall, > the amount of memory used is the same as today (so not really a waste per-se). > > It might be possible to limit that once we reworked the common code to know > the number of vCPUs earlier on (see discussion on patch #1). Yeah, this is nasty, but it is clear that until we rework the code to set d->max_vcpus earlier it won't get fixed. Nothing we can do here. So, I think ideally we would want to fix vgic_v3_rdist_count, but today we can't. Maybe we could rename vgic_v3_rdist_count to vgic_v3_hw_rdist_count, and add a short TODO comment somewhere in the file? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-27 23:34 ` Stefano Stabellini @ 2018-09-28 20:37 ` Julien Grall 2018-09-28 23:46 ` Stefano Stabellini 0 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-28 20:37 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, shameerali.kolothum.thodi, andre.przywara On 09/28/2018 12:34 AM, Stefano Stabellini wrote: > On Wed, 26 Sep 2018, Julien Grall wrote: >> Hi Stefano, >> >> On 09/25/2018 09:38 PM, Stefano Stabellini wrote: >>> On Tue, 4 Sep 2018, Julien Grall wrote: >>>> At the moment, Xen is assuming the hardware domain will have the same >>>> number of re-distributor regions as the host. However, as the >>>> number of CPUs or the stride (e.g on GICv4) may be different we end up >>>> exposing regions which does not contain any re-distributors. >>>> >>>> When booting, Linux will go through all the re-distributor region to >>>> check whether a property (e.g vPLIs) is available accross all the >>>> re-distributors. This will result to a data abort on empty regions >>>> because there are no underlying re-distributor. >>>> >>>> So we need to limit the number of regions exposed to the hardware >>>> domain. The code reworked to only expose the minimun number of regions >>>> required by the hardware domain. It is assumed the regions will be >>>> populated starting from the first one. >>> >>> I have a question: given that it is possible for a rdist_region to cover >>> more than 1 cpu, could we get into troubles if the last rdist_region of >>> the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? >>> get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write >>> would return 0. >>> This case seems to be handled correctly but I wanted to >>> double check. >> >> 0 means a data abort will be injected into the guest. However, the guest >> should never touch that because the last valid re-distributor of the regions >> will have GICR_TYPER.Last set. >> >> So the guest OS will stop looking for more re-distributors in that region. > > OK > > >>> > >>> I think we also need to fix vgic_v3_rdist_count? Today it just returns >>> vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it >>> unfixed? If we do that, we might be able to get rid of the modifications >>> to vgic_v3_real_domain_init. >> >> We don't want to fix vgic_v3_rdist_count. The helper returns the maximum >> re-distributors regions. > > We don't want to or we can't? Because it looks like we would want to fix > vgic_v3_rdist_count if we could, right? It is called from domain > specific initialization functions, so theoretically it should return > domain specific vgic information, not physical information. We don't want to fix in the current design. > > >> This is used to compute the number of IO handlers and >> allocating the array storing the regions. >> >> I am pretty sure you will say we will waste memory. However, at the moment, >> we need to know the number of IO handlers much before we know the number of >> vCPUs. For the array, we would need to go through the regions twice (regions >> may not be the same size so we can't infer easily the number needed). Overall, >> the amount of memory used is the same as today (so not really a waste per-se). >> >> It might be possible to limit that once we reworked the common code to know >> the number of vCPUs earlier on (see discussion on patch #1). > > Yeah, this is nasty, but it is clear that until we rework the code to > set d->max_vcpus earlier it won't get fixed. Nothing we can do here. > > So, I think ideally we would want to fix vgic_v3_rdist_count, but today > we can't. Maybe we could rename vgic_v3_rdist_count to > vgic_v3_hw_rdist_count, and add a short TODO comment somewhere in the > file? > Which would be wrong as the function don't always return the number of rdist for the HW. A better name would be vgic_v3_max_rdist_count(struct domain *d). Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions 2018-09-28 20:37 ` Julien Grall @ 2018-09-28 23:46 ` Stefano Stabellini 0 siblings, 0 replies; 24+ messages in thread From: Stefano Stabellini @ 2018-09-28 23:46 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Stefano Stabellini, shameerali.kolothum.thodi, andre.przywara On Fri, 28 Sep 2018, Julien Grall wrote: > On 09/28/2018 12:34 AM, Stefano Stabellini wrote: > > On Wed, 26 Sep 2018, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 09/25/2018 09:38 PM, Stefano Stabellini wrote: > > > > On Tue, 4 Sep 2018, Julien Grall wrote: > > > > > At the moment, Xen is assuming the hardware domain will have the same > > > > > number of re-distributor regions as the host. However, as the > > > > > number of CPUs or the stride (e.g on GICv4) may be different we end up > > > > > exposing regions which does not contain any re-distributors. > > > > > > > > > > When booting, Linux will go through all the re-distributor region to > > > > > check whether a property (e.g vPLIs) is available accross all the > > > > > re-distributors. This will result to a data abort on empty regions > > > > > because there are no underlying re-distributor. > > > > > > > > > > So we need to limit the number of regions exposed to the hardware > > > > > domain. The code reworked to only expose the minimun number of regions > > > > > required by the hardware domain. It is assumed the regions will be > > > > > populated starting from the first one. > > > > > > > > I have a question: given that it is possible for a rdist_region to cover > > > > more than 1 cpu, could we get into troubles if the last rdist_region of > > > > the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? > > > > get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write > > > > would return 0. > > > > This case seems to be handled correctly but I wanted to > > > > double check. > > > > > > 0 means a data abort will be injected into the guest. However, the guest > > > should never touch that because the last valid re-distributor of the > > > regions > > > will have GICR_TYPER.Last set. > > > > > > So the guest OS will stop looking for more re-distributors in that region. > > > > OK > > > > > > > > > > > > > I think we also need to fix vgic_v3_rdist_count? Today it just returns > > > > vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it > > > > unfixed? If we do that, we might be able to get rid of the modifications > > > > to vgic_v3_real_domain_init. > > > > > > We don't want to fix vgic_v3_rdist_count. The helper returns the maximum > > > re-distributors regions. > > > > We don't want to or we can't? Because it looks like we would want to fix > > vgic_v3_rdist_count if we could, right? It is called from domain > > specific initialization functions, so theoretically it should return > > domain specific vgic information, not physical information. > > We don't want to fix in the current design. > > > > > > > > This is used to compute the number of IO handlers and > > > allocating the array storing the regions. > > > > > > I am pretty sure you will say we will waste memory. However, at the > > > moment, > > > we need to know the number of IO handlers much before we know the number > > > of > > > vCPUs. For the array, we would need to go through the regions twice > > > (regions > > > may not be the same size so we can't infer easily the number needed). > > > Overall, > > > the amount of memory used is the same as today (so not really a waste > > > per-se). > > > > > > It might be possible to limit that once we reworked the common code to > > > know > > > the number of vCPUs earlier on (see discussion on patch #1). > > > > Yeah, this is nasty, but it is clear that until we rework the code to > > set d->max_vcpus earlier it won't get fixed. Nothing we can do here. > > > > So, I think ideally we would want to fix vgic_v3_rdist_count, but today > > we can't. Maybe we could rename vgic_v3_rdist_count to > > vgic_v3_hw_rdist_count, and add a short TODO comment somewhere in the > > file? > > > > Which would be wrong as the function don't always return the number of rdist > for the HW. > > A better name would be vgic_v3_max_rdist_count(struct domain *d). I am OK with that _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent 2018-09-04 19:21 [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Julien Grall 2018-09-04 19:21 ` [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall 2018-09-04 19:21 ` [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall @ 2018-09-04 19:21 ` Julien Grall 2018-09-25 20:08 ` Stefano Stabellini 2018-09-06 15:49 ` [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Shameerali Kolothum Thodi 3 siblings, 1 reply; 24+ messages in thread From: Julien Grall @ 2018-09-04 19:21 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, sstabellini, shameerali.kolothum.thodi, Andrew Cooper vgic_v3_its_free_domain may be called before vgic_v3_its_init_domain if the vGIC was failing to initalize itself. This means the list would be unitialized and result in a crash. Thankfully, we only allow ITS for the hardware domain. So the crash is not a security issue. Fix it by checking whether the list the NULL. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/arm/vgic-v3-its.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c index 32061c6b03..9edd97c4e7 100644 --- a/xen/arch/arm/vgic-v3-its.c +++ b/xen/arch/arm/vgic-v3-its.c @@ -1548,6 +1548,10 @@ void vgic_v3_its_free_domain(struct domain *d) { struct virt_its *pos, *temp; + /* Cope with unitialized vITS */ + if ( list_head_is_null(&d->arch.vgic.vits_list) ) + return; + list_for_each_entry_safe( pos, temp, &d->arch.vgic.vits_list, vits_list ) { list_del(&pos->vits_list); -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent 2018-09-04 19:21 ` [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent Julien Grall @ 2018-09-25 20:08 ` Stefano Stabellini 0 siblings, 0 replies; 24+ messages in thread From: Stefano Stabellini @ 2018-09-25 20:08 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, sstabellini, Andrew Cooper, shameerali.kolothum.thodi, andre.przywara On Tue, 4 Sep 2018, Julien Grall wrote: > vgic_v3_its_free_domain may be called before vgic_v3_its_init_domain if > the vGIC was failing to initalize itself. This means the list would be > unitialized and result in a crash. > > Thankfully, we only allow ITS for the hardware domain. So the crash is > not a security issue. Fix it by checking whether the list the NULL. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > --- > xen/arch/arm/vgic-v3-its.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > index 32061c6b03..9edd97c4e7 100644 > --- a/xen/arch/arm/vgic-v3-its.c > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -1548,6 +1548,10 @@ void vgic_v3_its_free_domain(struct domain *d) > { > struct virt_its *pos, *temp; > > + /* Cope with unitialized vITS */ > + if ( list_head_is_null(&d->arch.vgic.vits_list) ) > + return; > + > list_for_each_entry_safe( pos, temp, &d->arch.vgic.vits_list, vits_list ) > { > list_del(&pos->vits_list); > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] xen/arm: vgic-v3: Bug fixes 2018-09-04 19:21 [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Julien Grall ` (2 preceding siblings ...) 2018-09-04 19:21 ` [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent Julien Grall @ 2018-09-06 15:49 ` Shameerali Kolothum Thodi 3 siblings, 0 replies; 24+ messages in thread From: Shameerali Kolothum Thodi @ 2018-09-06 15:49 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: andre.przywara, sstabellini > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 04 September 2018 20:22 > To: xen-devel@lists.xenproject.org > Cc: sstabellini@kernel.org; andre.przywara@arm.com; Shameerali Kolothum > Thodi <shameerali.kolothum.thodi@huawei.com>; Julien Grall > <julien.grall@arm.com> > Subject: [PATCH 0/3] xen/arm: vgic-v3: Bug fixes > > Hi all, > > The first 2 patches of the series are meant to address Dom0 boot failure when > on GICv4 platforms as well as when the number of vCPUs is not equal to the > numbers of pCPUs. They should be backported up Xen 4.8. Tested first two patches on this series on ARM64 GICv4 platform(ACPI boot) and the reported issue[1] is no more present. Please feel free to add, Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Thanks, Shameer [1] https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg00138.html > Patch #3 should address failure when failing to create guest. ITS is still under > EXPERT, so I would not consider it as backport. > > Cheers, > > Julien Grall (3): > [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the > domain information > xen/arm: vgic-v3: Don't create empty re-distributor regions > xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent > > xen/arch/arm/gic-v3.c | 10 ++++++++-- > xen/arch/arm/vgic-v3-its.c | 4 ++++ > xen/arch/arm/vgic-v3.c | 40 ++++++++++++++++++++++++++++++++++++++- > - > 3 files changed, 50 insertions(+), 4 deletions(-) > > -- > 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-10-01 11:31 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-04 19:21 [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Julien Grall 2018-09-04 19:21 ` [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information Julien Grall 2018-09-04 19:35 ` Julien Grall 2018-09-04 19:53 ` Andrew Cooper 2018-09-05 13:25 ` Julien Grall 2018-09-25 20:45 ` Stefano Stabellini 2018-09-26 20:14 ` Julien Grall 2018-09-27 23:11 ` Stefano Stabellini 2018-09-28 20:35 ` Julien Grall 2018-09-28 23:38 ` Andrew Cooper 2018-09-28 23:45 ` Stefano Stabellini 2018-09-28 23:48 ` Andrew Cooper 2018-10-01 9:43 ` Julien Grall 2018-10-01 9:53 ` Andrew Cooper 2018-10-01 11:31 ` Julien Grall 2018-09-04 19:21 ` [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions Julien Grall 2018-09-25 20:38 ` Stefano Stabellini 2018-09-26 20:36 ` Julien Grall 2018-09-27 23:34 ` Stefano Stabellini 2018-09-28 20:37 ` Julien Grall 2018-09-28 23:46 ` Stefano Stabellini 2018-09-04 19:21 ` [PATCH 3/3] xen/arm: vgic-v3-its: Make vgic_v3_its_free_domain idempotent Julien Grall 2018-09-25 20:08 ` Stefano Stabellini 2018-09-06 15:49 ` [PATCH 0/3] xen/arm: vgic-v3: Bug fixes Shameerali Kolothum Thodi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.