| Message ID | 20220628143100.3228092-5-schnelle@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Rework pci_scan_slot() and isolated PCI functions | expand |
On 6/28/22 16:30, Niklas Schnelle wrote: > Like the jailhouse hypervisor s390's PCI architecture allows passing > isolated PCI functions to an OS instance. As of now this is was not > utilized even with multi-function support as the s390 PCI code makes > sure that only virtual PCI busses including a function with devfn 0 are > presented to the PCI subsystem. A subsequent change will remove this > restriction. > > Allow probing such functions by replacing the existing check for > jailhouse_paravirt() with a new hypervisor_isolated_pci_functions() > helper. > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > drivers/pci/probe.c | 2 +- > include/linux/hypervisor.h | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index a18e07e6a7df..156dd13594b8 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2667,7 +2667,7 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > * a hypervisor which passes through individual PCI > * functions. > */ > - if (!jailhouse_paravirt()) > + if (!hypervisor_isolated_pci_functions()) > break; > } > fn = next_fn(bus, dev, fn); > diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h > index fc08b433c856..33b1c0482aac 100644 > --- a/include/linux/hypervisor.h > +++ b/include/linux/hypervisor.h > @@ -32,4 +32,12 @@ static inline bool jailhouse_paravirt(void) > > #endif /* !CONFIG_X86 */ > > +static inline bool hypervisor_isolated_pci_functions(void) > +{ > + if (IS_ENABLED(CONFIG_S390)) > + return true; > + else > + return jailhouse_paravirt(); I would spare the else, Another remark, shouldn't it be the last patch? otherwise LGTM Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> > +} > + > #endif /* __LINUX_HYPEVISOR_H */ >
On Thu, 2022-06-30 at 14:45 +0200, Pierre Morel wrote: > > On 6/28/22 16:30, Niklas Schnelle wrote: > > Like the jailhouse hypervisor s390's PCI architecture allows passing > > isolated PCI functions to an OS instance. As of now this is was not > > utilized even with multi-function support as the s390 PCI code makes > > sure that only virtual PCI busses including a function with devfn 0 are > > presented to the PCI subsystem. A subsequent change will remove this > > restriction. > > > > Allow probing such functions by replacing the existing check for > > jailhouse_paravirt() with a new hypervisor_isolated_pci_functions() > > helper. > > > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > > --- > > drivers/pci/probe.c | 2 +- > > include/linux/hypervisor.h | 8 ++++++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index a18e07e6a7df..156dd13594b8 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -2667,7 +2667,7 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > > * a hypervisor which passes through individual PCI > > * functions. > > */ > > - if (!jailhouse_paravirt()) > > + if (!hypervisor_isolated_pci_functions()) > > break; > > } > > fn = next_fn(bus, dev, fn); > > diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h > > index fc08b433c856..33b1c0482aac 100644 > > --- a/include/linux/hypervisor.h > > +++ b/include/linux/hypervisor.h > > @@ -32,4 +32,12 @@ static inline bool jailhouse_paravirt(void) > > > > #endif /* !CONFIG_X86 */ > > > > +static inline bool hypervisor_isolated_pci_functions(void) > > +{ > > + if (IS_ENABLED(CONFIG_S390)) > > + return true; > > + else > > + return jailhouse_paravirt(); > > I would spare the else, I don't have a preference for either style so sure. > > Another remark, shouldn't it be the last patch? Either way should work. Without the last patch we don't try to probe and without this patch the probing wouldn't find the function. I think I'll keep the order to keep the PCI subsystem changes together and because I feel trying to probe without that working is worse than not probing. > > otherwise LGTM > > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> Thanks for taking a look! > > > > +} > > + > > #endif /* __LINUX_HYPEVISOR_H */ > >
On Tue, Jun 28, 2022 at 04:30:59PM +0200, Niklas Schnelle wrote: > Like the jailhouse hypervisor s390's PCI architecture allows passing > isolated PCI functions to an OS instance. As of now this is was not > utilized even with multi-function support as the s390 PCI code makes > sure that only virtual PCI busses including a function with devfn 0 are > presented to the PCI subsystem. A subsequent change will remove this > restriction. > > Allow probing such functions by replacing the existing check for > jailhouse_paravirt() with a new hypervisor_isolated_pci_functions() > helper. > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > drivers/pci/probe.c | 2 +- > include/linux/hypervisor.h | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index a18e07e6a7df..156dd13594b8 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2667,7 +2667,7 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > * a hypervisor which passes through individual PCI > * functions. > */ > - if (!jailhouse_paravirt()) > + if (!hypervisor_isolated_pci_functions()) > break; > } > fn = next_fn(bus, dev, fn); > diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h > index fc08b433c856..33b1c0482aac 100644 > --- a/include/linux/hypervisor.h > +++ b/include/linux/hypervisor.h > @@ -32,4 +32,12 @@ static inline bool jailhouse_paravirt(void) > > #endif /* !CONFIG_X86 */ > > +static inline bool hypervisor_isolated_pci_functions(void) > +{ > + if (IS_ENABLED(CONFIG_S390)) > + return true; > + else > + return jailhouse_paravirt(); It looks kind of wasteful that jailhouse_paravirt() searches the DT for "jailhouse,cell" several times when I think that's an unchanging property. Obviously you didn't add that in this series, and s390 avoids that cost anyway. But the jailhouse folks might consider optimizing it somehow. > +} > + > #endif /* __LINUX_HYPEVISOR_H */ > -- > 2.32.0 >
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index a18e07e6a7df..156dd13594b8 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2667,7 +2667,7 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) * a hypervisor which passes through individual PCI * functions. */ - if (!jailhouse_paravirt()) + if (!hypervisor_isolated_pci_functions()) break; } fn = next_fn(bus, dev, fn); diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h index fc08b433c856..33b1c0482aac 100644 --- a/include/linux/hypervisor.h +++ b/include/linux/hypervisor.h @@ -32,4 +32,12 @@ static inline bool jailhouse_paravirt(void) #endif /* !CONFIG_X86 */ +static inline bool hypervisor_isolated_pci_functions(void) +{ + if (IS_ENABLED(CONFIG_S390)) + return true; + else + return jailhouse_paravirt(); +} + #endif /* __LINUX_HYPEVISOR_H */
Like the jailhouse hypervisor s390's PCI architecture allows passing isolated PCI functions to an OS instance. As of now this is was not utilized even with multi-function support as the s390 PCI code makes sure that only virtual PCI busses including a function with devfn 0 are presented to the PCI subsystem. A subsequent change will remove this restriction. Allow probing such functions by replacing the existing check for jailhouse_paravirt() with a new hypervisor_isolated_pci_functions() helper. Cc: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> --- drivers/pci/probe.c | 2 +- include/linux/hypervisor.h | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)