| Message ID | e279636ea47b7d06056c2f70e76900b8d0b30ee9.1629366665.git.rahul.singh@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI devices passthrough on Arm | expand |
On 19.08.2021 14:02, Rahul Singh wrote: > Add cmdline boot option "pci=on" to enable/disable the PCI init during > boot. > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> > --- > xen/arch/arm/pci/pci.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) Any addition or change of a command line option should be accompanied by an adjustment to docs/misc/xen-command-line.pandoc, please. Jan
Hi Rahul, On 19/08/2021 13:02, Rahul Singh wrote: > Add cmdline boot option "pci=on" to enable/disable the PCI init during > boot. I read this as "PCI" will be either disabled/enabled for the platform. Whereas, I think it will be used to decide whether Xen discover PCI and PCI passthrough is supported or not. Can you also clarify why a user would want to select "pci=off"? Cheers,
Hi Julien, > On 19 Aug 2021, at 1:31 pm, Julien Grall <julien@xen.org> wrote: > > Hi Rahul, > > On 19/08/2021 13:02, Rahul Singh wrote: >> Add cmdline boot option "pci=on" to enable/disable the PCI init during >> boot. > > I read this as "PCI" will be either disabled/enabled for the platform. Whereas, I think it will be used to decide whether Xen discover PCI and PCI passthrough is supported or not. Yes. I will modify the option to "pci-passthrough== <boolean>" > > Can you also clarify why a user would want to select "pci=off"? As pci-passthrough support emulate the PCI devices for DOM0 also, I thought if someone want to boot the DOM0 without emulating the PCI device in XEN and wants to have direct access to device. I am ok to drop this patch if you feel adding the option is not required at all. Regards, Rahul
On 20/08/2021 13:19, Rahul Singh wrote: > Hi Julien, Hi Rahul, > >> On 19 Aug 2021, at 1:31 pm, Julien Grall <julien@xen.org> wrote: >> >> Hi Rahul, >> >> On 19/08/2021 13:02, Rahul Singh wrote: >>> Add cmdline boot option "pci=on" to enable/disable the PCI init during >>> boot. >> >> I read this as "PCI" will be either disabled/enabled for the platform. Whereas, I think it will be used to decide whether Xen discover PCI and PCI passthrough is supported or not. > > Yes. I will modify the option to "pci-passthrough== <boolean>" >> >> Can you also clarify why a user would want to select "pci=off"? > > As pci-passthrough support emulate the PCI devices for DOM0 also, I thought if someone want to > boot the DOM0 without emulating the PCI device in XEN and wants to have direct access to device. Dom0 will always have direct access to the PCI device. The only difference is whether the access to the hostbridge and config space will be trapped by Xen. I expect the both to mainly happen during boot and therefore the overhead will be limited. > > I am ok to drop this patch if you feel adding the option is not required at all. One of the reason I could see this option to be useful is to figure out if an issue occurs because of the hostbridge emulation. Yet, I am still not fully convinced adding an option is worth it. Jan and others, any opinions? Cheers,
On 20.08.2021 16:34, Julien Grall wrote: > On 20/08/2021 13:19, Rahul Singh wrote: >>> On 19 Aug 2021, at 1:31 pm, Julien Grall <julien@xen.org> wrote: >>> On 19/08/2021 13:02, Rahul Singh wrote: >>>> Add cmdline boot option "pci=on" to enable/disable the PCI init during >>>> boot. >>> >>> I read this as "PCI" will be either disabled/enabled for the platform. Whereas, I think it will be used to decide whether Xen discover PCI and PCI passthrough is supported or not. >> >> Yes. I will modify the option to "pci-passthrough== <boolean>" >>> >>> Can you also clarify why a user would want to select "pci=off"? >> >> As pci-passthrough support emulate the PCI devices for DOM0 also, I thought if someone want to >> boot the DOM0 without emulating the PCI device in XEN and wants to have direct access to device. > > Dom0 will always have direct access to the PCI device. The only > difference is whether the access to the hostbridge and config space will > be trapped by Xen. I expect the both to mainly happen during boot and > therefore the overhead will be limited. > >> >> I am ok to drop this patch if you feel adding the option is not required at all. > One of the reason I could see this option to be useful is to figure out > if an issue occurs because of the hostbridge emulation. Yet, I am still > not fully convinced adding an option is worth it. > > Jan and others, any opinions? Well, if there's a proper fallback, then why not allow using it in case of problems? Jan
On Fri, 20 Aug 2021, Jan Beulich wrote: > On 20.08.2021 16:34, Julien Grall wrote: > > On 20/08/2021 13:19, Rahul Singh wrote: > >>> On 19 Aug 2021, at 1:31 pm, Julien Grall <julien@xen.org> wrote: > >>> On 19/08/2021 13:02, Rahul Singh wrote: > >>>> Add cmdline boot option "pci=on" to enable/disable the PCI init during > >>>> boot. > >>> > >>> I read this as "PCI" will be either disabled/enabled for the platform. Whereas, I think it will be used to decide whether Xen discover PCI and PCI passthrough is supported or not. > >> > >> Yes. I will modify the option to "pci-passthrough== <boolean>" > >>> > >>> Can you also clarify why a user would want to select "pci=off"? > >> > >> As pci-passthrough support emulate the PCI devices for DOM0 also, I thought if someone want to > >> boot the DOM0 without emulating the PCI device in XEN and wants to have direct access to device. > > > > Dom0 will always have direct access to the PCI device. The only > > difference is whether the access to the hostbridge and config space will > > be trapped by Xen. I expect the both to mainly happen during boot and > > therefore the overhead will be limited. > > > >> > >> I am ok to drop this patch if you feel adding the option is not required at all. > > One of the reason I could see this option to be useful is to figure out > > if an issue occurs because of the hostbridge emulation. Yet, I am still > > not fully convinced adding an option is worth it. > > > > Jan and others, any opinions? > > Well, if there's a proper fallback, then why not allow using it in > case of problems? I think it would be good to have the option, if nothing else for debugging.
On Thu, 19 Aug 2021, Rahul Singh wrote: > Add cmdline boot option "pci=on" to enable/disable the PCI init during > boot. > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> > --- > xen/arch/arm/pci/pci.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c > index d1c9cf997d..dc63bbc2a2 100644 > --- a/xen/arch/arm/pci/pci.c > +++ b/xen/arch/arm/pci/pci.c > @@ -62,8 +62,38 @@ static void __init acpi_pci_init(void) > static inline void __init acpi_pci_init(void) { } > #endif > > +static bool __initdata param_pci_enable; > + > +static int __init parse_pci_param(const char *arg) > +{ > + if ( !arg ) > + { > + param_pci_enable = false; > + return 0; > + } > + > + switch ( parse_bool(arg, NULL) ) > + { > + case 0: > + param_pci_enable = false; > + return 0; > + case 1: > + param_pci_enable = true; > + return 0; > + } > + > + return -EINVAL; > +} > +custom_param("pci", parse_pci_param); Consider using boolean_param instead. It supports "on". > static int __init pci_init(void) > { > + /* > + * Enable PCI when has been enabled explicitly (pci=on) > + */ > + if ( !param_pci_enable) > + return 0; > + > if ( acpi_disabled ) > dt_pci_init(); > else > -- > 2.17.1 >
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c index d1c9cf997d..dc63bbc2a2 100644 --- a/xen/arch/arm/pci/pci.c +++ b/xen/arch/arm/pci/pci.c @@ -62,8 +62,38 @@ static void __init acpi_pci_init(void) static inline void __init acpi_pci_init(void) { } #endif +static bool __initdata param_pci_enable; + +static int __init parse_pci_param(const char *arg) +{ + if ( !arg ) + { + param_pci_enable = false; + return 0; + } + + switch ( parse_bool(arg, NULL) ) + { + case 0: + param_pci_enable = false; + return 0; + case 1: + param_pci_enable = true; + return 0; + } + + return -EINVAL; +} +custom_param("pci", parse_pci_param); + static int __init pci_init(void) { + /* + * Enable PCI when has been enabled explicitly (pci=on) + */ + if ( !param_pci_enable) + return 0; + if ( acpi_disabled ) dt_pci_init(); else
Add cmdline boot option "pci=on" to enable/disable the PCI init during boot. Signed-off-by: Rahul Singh <rahul.singh@arm.com> --- xen/arch/arm/pci/pci.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)