| Message ID | 20190821035315.12812-7-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/8] xen/arm: introduce handle_device_interrupts | expand |
Hi Stefano, On 8/21/19 4:53 AM, Stefano Stabellini wrote: > We don't have a clear way to know how many virtual SPIs we need for the > dom0-less domains. Introduce a new option under xen,domain to specify > the number of SPIs to allocate for a domain. > > The property is optional. When absent, we'll use the physical number of > GIC lines for dom0-less domains, just like for dom0. Given that > dom0-less VMs are meant for static partitioning scenarios where the > number of VMs is very low, increased memory overhead should not be a > problem, and it is possible to minimize it using "nr_spis". > > Remove the old setting of nr_spis based on the presence of the vpl011. I am afraid this still does not explain the implications of this patch to current setup (with and without VPL011). For instance, with your change, VPL011 may not work anymore. Imagine we decide to push the vpl011 interrupt towards the end of the Interrupt ID space (i.e. 1019). I don't think we want the user to have to select nr_spis by himself for this case. Regarding the change without vpl011, this is not explained why all the domains (even the one without SPIs routed) will have SPIs exposed. For instance, if you were to expose 256 interrupts for 4 domains, this will roughly use 80KB of memory. I don't think this is what you had in mind as "low footprint". Cheers,
On Wed, 11 Sep 2019, Julien Grall wrote: > Hi Stefano, > > On 8/21/19 4:53 AM, Stefano Stabellini wrote: > > We don't have a clear way to know how many virtual SPIs we need for the > > dom0-less domains. Introduce a new option under xen,domain to specify > > the number of SPIs to allocate for a domain. > > > > The property is optional. When absent, we'll use the physical number of > > GIC lines for dom0-less domains, just like for dom0. Given that > > dom0-less VMs are meant for static partitioning scenarios where the > > number of VMs is very low, increased memory overhead should not be a > > problem, and it is possible to minimize it using "nr_spis". > > > > Remove the old setting of nr_spis based on the presence of the vpl011. > > I am afraid this still does not explain the implications of this patch to > current setup (with and without VPL011). > > For instance, with your change, VPL011 may not work anymore. Imagine we decide > to push the vpl011 interrupt towards the end of the Interrupt ID space (i.e. > 1019). > > I don't think we want the user to have to select nr_spis by himself for this > case. > > Regarding the change without vpl011, this is not explained why all the domains > (even the one without SPIs routed) will have SPIs exposed. For instance, if > you were to expose 256 interrupts for 4 domains, this will roughly use 80KB of > memory. I don't think this is what you had in mind as "low footprint". What do you think of the following: The implication of this change is that without nr_spis dom0less domains get the same amount of SPI allocated as dom0, regardless of how many physical devices they have assigned, and regardless of whether they have a virtual pl011 (which also needs an emulated SPI). When nr_spis is present, the domain gets exactly nr_spis allocated SPIs. If the number is too low, it might not be enough for the devices assigned it to it. If the number is less than GUEST_VPL011_SPI, the virtual pl011 won't work.
Hi, On 24/09/2019 18:56, Stefano Stabellini wrote: > On Wed, 11 Sep 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 8/21/19 4:53 AM, Stefano Stabellini wrote: >>> We don't have a clear way to know how many virtual SPIs we need for the >>> dom0-less domains. Introduce a new option under xen,domain to specify >>> the number of SPIs to allocate for a domain. >>> >>> The property is optional. When absent, we'll use the physical number of >>> GIC lines for dom0-less domains, just like for dom0. Given that >>> dom0-less VMs are meant for static partitioning scenarios where the >>> number of VMs is very low, increased memory overhead should not be a >>> problem, and it is possible to minimize it using "nr_spis". >>> >>> Remove the old setting of nr_spis based on the presence of the vpl011. >> >> I am afraid this still does not explain the implications of this patch to >> current setup (with and without VPL011). >> >> For instance, with your change, VPL011 may not work anymore. Imagine we decide >> to push the vpl011 interrupt towards the end of the Interrupt ID space (i.e. >> 1019). >> >> I don't think we want the user to have to select nr_spis by himself for this >> case. >> >> Regarding the change without vpl011, this is not explained why all the domains >> (even the one without SPIs routed) will have SPIs exposed. For instance, if >> you were to expose 256 interrupts for 4 domains, this will roughly use 80KB of >> memory. I don't think this is what you had in mind as "low footprint". > > What do you think of the following: > > The implication of this change is that without nr_spis dom0less domains > get the same amount of SPI allocated as dom0, regardless of how many > physical devices they have assigned, and regardless of whether they have > a virtual pl011 (which also needs an emulated SPI). > > When nr_spis is present, the domain gets exactly nr_spis allocated SPIs. > If the number is too low, it might not be enough for the devices > assigned it to it. If the number is less than GUEST_VPL011_SPI, the > virtual pl011 won't work. This is good to address my first remark. How about the others? Cheers,
On Wed, 25 Sep 2019, Julien Grall wrote: > Hi, > > On 24/09/2019 18:56, Stefano Stabellini wrote: > > On Wed, 11 Sep 2019, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 8/21/19 4:53 AM, Stefano Stabellini wrote: > > > > We don't have a clear way to know how many virtual SPIs we need for the > > > > dom0-less domains. Introduce a new option under xen,domain to specify > > > > the number of SPIs to allocate for a domain. > > > > > > > > The property is optional. When absent, we'll use the physical number of > > > > GIC lines for dom0-less domains, just like for dom0. Given that > > > > dom0-less VMs are meant for static partitioning scenarios where the > > > > number of VMs is very low, increased memory overhead should not be a > > > > problem, and it is possible to minimize it using "nr_spis". > > > > > > > > Remove the old setting of nr_spis based on the presence of the vpl011. > > > > > > I am afraid this still does not explain the implications of this patch to > > > current setup (with and without VPL011). > > > > > > For instance, with your change, VPL011 may not work anymore. Imagine we > > > decide > > > to push the vpl011 interrupt towards the end of the Interrupt ID space > > > (i.e. > > > 1019). > > > > > > I don't think we want the user to have to select nr_spis by himself for > > > this > > > case. > > > > > > Regarding the change without vpl011, this is not explained why all the > > > domains > > > (even the one without SPIs routed) will have SPIs exposed. For instance, > > > if > > > you were to expose 256 interrupts for 4 domains, this will roughly use > > > 80KB of > > > memory. I don't think this is what you had in mind as "low footprint". > > What do you think of the following: > > > > The implication of this change is that without nr_spis dom0less domains > > get the same amount of SPI allocated as dom0, regardless of how many > > physical devices they have assigned, and regardless of whether they have > > a virtual pl011 (which also needs an emulated SPI). > > > > When nr_spis is present, the domain gets exactly nr_spis allocated SPIs. > > If the number is too low, it might not be enough for the devices > > assigned it to it. If the number is less than GUEST_VPL011_SPI, the > > virtual pl011 won't work. > > This is good to address my first remark. How about the others? For your point about "VPL011 may not work anymore", are you suggesting we print a warning or that we always allocate at least GUEST_VPL011_SPI+1 SPIs if vpl011 is requested? For your point about "it is not explained why all the domains will have SPIs exposed" would you like me to add a sentence to the commit message to make it clearer or do you have something else in mind?
Hi, On 25/09/2019 17:16, Stefano Stabellini wrote: > On Wed, 25 Sep 2019, Julien Grall wrote: >> Hi, >> >> On 24/09/2019 18:56, Stefano Stabellini wrote: >>> On Wed, 11 Sep 2019, Julien Grall wrote: >>>> Hi Stefano, >>>> >>>> On 8/21/19 4:53 AM, Stefano Stabellini wrote: >>>>> We don't have a clear way to know how many virtual SPIs we need for the >>>>> dom0-less domains. Introduce a new option under xen,domain to specify >>>>> the number of SPIs to allocate for a domain. >>>>> >>>>> The property is optional. When absent, we'll use the physical number of >>>>> GIC lines for dom0-less domains, just like for dom0. Given that >>>>> dom0-less VMs are meant for static partitioning scenarios where the >>>>> number of VMs is very low, increased memory overhead should not be a >>>>> problem, and it is possible to minimize it using "nr_spis". >>>>> >>>>> Remove the old setting of nr_spis based on the presence of the vpl011. >>>> >>>> I am afraid this still does not explain the implications of this patch to >>>> current setup (with and without VPL011). >>>> >>>> For instance, with your change, VPL011 may not work anymore. Imagine we >>>> decide >>>> to push the vpl011 interrupt towards the end of the Interrupt ID space >>>> (i.e. >>>> 1019). >>>> >>>> I don't think we want the user to have to select nr_spis by himself for >>>> this >>>> case. >>>> >>>> Regarding the change without vpl011, this is not explained why all the >>>> domains >>>> (even the one without SPIs routed) will have SPIs exposed. For instance, >>>> if >>>> you were to expose 256 interrupts for 4 domains, this will roughly use >>>> 80KB of >>>> memory. I don't think this is what you had in mind as "low footprint". >>> What do you think of the following: >>> >>> The implication of this change is that without nr_spis dom0less domains >>> get the same amount of SPI allocated as dom0, regardless of how many >>> physical devices they have assigned, and regardless of whether they have >>> a virtual pl011 (which also needs an emulated SPI). >>> >>> When nr_spis is present, the domain gets exactly nr_spis allocated SPIs. >>> If the number is too low, it might not be enough for the devices >>> assigned it to it. If the number is less than GUEST_VPL011_SPI, the >>> virtual pl011 won't work. >> >> This is good to address my first remark. How about the others? > > For your point about "VPL011 may not work anymore", are you suggesting > we print a warning or that we always allocate at least > GUEST_VPL011_SPI+1 SPIs if vpl011 is requested? My preference is to do the later as this match the behavior when guest are created by libxl. This is also the current behavior. But if you want to change this behavior, then you need to document it as this is a breakage between the previous interface. > > For your point about "it is not explained why all the domains will have > SPIs exposed" would you like me to add a sentence to the commit message > to make it clearer or do you have something else in mind? A sentence in the commit message is be good enough. Cheers,
On Wed, 25 Sep 2019, Julien Grall wrote: > Hi, > > On 25/09/2019 17:16, Stefano Stabellini wrote: > > On Wed, 25 Sep 2019, Julien Grall wrote: > > > Hi, > > > > > > On 24/09/2019 18:56, Stefano Stabellini wrote: > > > > On Wed, 11 Sep 2019, Julien Grall wrote: > > > > > Hi Stefano, > > > > > > > > > > On 8/21/19 4:53 AM, Stefano Stabellini wrote: > > > > > > We don't have a clear way to know how many virtual SPIs we need for > > > > > > the > > > > > > dom0-less domains. Introduce a new option under xen,domain to > > > > > > specify > > > > > > the number of SPIs to allocate for a domain. > > > > > > > > > > > > The property is optional. When absent, we'll use the physical number > > > > > > of > > > > > > GIC lines for dom0-less domains, just like for dom0. Given that > > > > > > dom0-less VMs are meant for static partitioning scenarios where the > > > > > > number of VMs is very low, increased memory overhead should not be a > > > > > > problem, and it is possible to minimize it using "nr_spis". > > > > > > > > > > > > Remove the old setting of nr_spis based on the presence of the > > > > > > vpl011. > > > > > > > > > > I am afraid this still does not explain the implications of this patch > > > > > to > > > > > current setup (with and without VPL011). > > > > > > > > > > For instance, with your change, VPL011 may not work anymore. Imagine > > > > > we > > > > > decide > > > > > to push the vpl011 interrupt towards the end of the Interrupt ID space > > > > > (i.e. > > > > > 1019). > > > > > > > > > > I don't think we want the user to have to select nr_spis by himself > > > > > for > > > > > this > > > > > case. > > > > > > > > > > Regarding the change without vpl011, this is not explained why all the > > > > > domains > > > > > (even the one without SPIs routed) will have SPIs exposed. For > > > > > instance, > > > > > if > > > > > you were to expose 256 interrupts for 4 domains, this will roughly use > > > > > 80KB of > > > > > memory. I don't think this is what you had in mind as "low footprint". > > > > What do you think of the following: > > > > > > > > The implication of this change is that without nr_spis dom0less domains > > > > get the same amount of SPI allocated as dom0, regardless of how many > > > > physical devices they have assigned, and regardless of whether they have > > > > a virtual pl011 (which also needs an emulated SPI). > > > > > > > > When nr_spis is present, the domain gets exactly nr_spis allocated SPIs. > > > > If the number is too low, it might not be enough for the devices > > > > assigned it to it. If the number is less than GUEST_VPL011_SPI, the > > > > virtual pl011 won't work. > > > > > > This is good to address my first remark. How about the others? > > For your point about "VPL011 may not work anymore", are you suggesting > > we print a warning or that we always allocate at least > > GUEST_VPL011_SPI+1 SPIs if vpl011 is requested? > > My preference is to do the later as this match the behavior when guest are > created by libxl. This is also the current behavior. OK, if nr_spis is not present, I'll make sure that the allocated SPIs are enough to account for GUEST_VPL011_SPI. > But if you want to change this behavior, then you need to document it as this > is a breakage between the previous interface. > > > > > For your point about "it is not explained why all the domains will have > > SPIs exposed" would you like me to add a sentence to the commit message > > to make it clearer or do you have something else in mind? > > A sentence in the commit message is be good enough. OK
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 256c83462a..86cbe4e67b 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2280,7 +2280,6 @@ void __init create_domUs(void) struct domain *d; struct xen_domctl_createdomain d_cfg = { .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE, - .arch.nr_spis = 0, .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap, .max_evtchn_port = -1, .max_grant_frames = 64, @@ -2290,13 +2289,13 @@ void __init create_domUs(void) if ( !dt_device_is_compatible(node, "xen,domain") ) continue; - if ( dt_property_read_bool(node, "vpl011") ) - d_cfg.arch.nr_spis = GUEST_VPL011_SPI - 32 + 1; - if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) ) panic("Missing property 'cpus' for domain %s\n", dt_node_name(node)); + if ( !dt_property_read_u32(node, "nr_spis", &d_cfg.arch.nr_spis) ) + d_cfg.arch.nr_spis = gic_number_lines() - 32; + d = domain_create(++max_init_domid, &d_cfg, false); if ( IS_ERR(d) ) panic("Error creating domain %s\n", dt_node_name(node));