| Message ID | 20220719174253.541965-2-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI devices passthrough on Arm, part 3 | expand |
Hello Rahul On 19.07.22 20:42, Oleksandr Tyshchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > Add a stub for is_memory_hole which is required for PCI passthrough > on Arm. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > --- > OT: It looks like the discussion got stuck. As I understand this > patch is not immediately needed in the context of current series > as PCI passthrough is not enabled on Arm at the moment. So the patch > could be added later on, but it is needed to allow PCI passthrough > to be built on Arm for those who want to test it. > > Copy here some context provided by Julien: > > Here a summary of the discussion (+ some my follow-up thoughts): > > is_memory_hole() was recently introduced on x86 (see commit 75cc460a1b8c > "xen/pci: detect when BARs are not suitably positioned") to check > whether the BAR are positioned outside of a valid memory range. This was > introduced to work-around quirky firmware. > > In theory, this could also happen on Arm. In practice, this may not > happen but it sounds better to sanity check that the BAR contains > "valid" I/O range. > > On x86, this is implemented by checking the region is not described is > in the e820. IIUC, on Arm, the BARs have to be positioned in pre-defined > ranges. So I think it would be possible to implement is_memory_hole() by > going through the list of hostbridges and check the ranges. > > But first, I'd like to confirm my understanding with Rahul, and others. May I please ask about your opinion on that? > > If we were going to go this route, I would also rename the function to > be better match what it is doing (i.e. it checks the BAR is correctly > placed). As a potentially optimization/hardening for Arm, we could pass > the hostbridge so we don't have to walk all of them. > --- > xen/arch/arm/mm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 009b8cd9ef..bb34b97eb5 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1708,6 +1708,12 @@ unsigned long get_upper_mfn_bound(void) > return max_page - 1; > } > > +bool is_memory_hole(mfn_t start, mfn_t end) > +{ > + /* TODO: this needs to be properly implemented. */ > + return true; > +} > + > /* > * Local variables: > * mode: C
Hi Oleksandr, > On 29 Jul 2022, at 5:28 pm, Oleksandr <olekstysh@gmail.com> wrote: > > > Hello Rahul > > > On 19.07.22 20:42, Oleksandr Tyshchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> Add a stub for is_memory_hole which is required for PCI passthrough >> on Arm. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> --- >> OT: It looks like the discussion got stuck. As I understand this >> patch is not immediately needed in the context of current series >> as PCI passthrough is not enabled on Arm at the moment. So the patch >> could be added later on, but it is needed to allow PCI passthrough >> to be built on Arm for those who want to test it. >> >> Copy here some context provided by Julien: >> >> Here a summary of the discussion (+ some my follow-up thoughts): >> >> is_memory_hole() was recently introduced on x86 (see commit 75cc460a1b8c >> "xen/pci: detect when BARs are not suitably positioned") to check >> whether the BAR are positioned outside of a valid memory range. This was >> introduced to work-around quirky firmware. >> >> In theory, this could also happen on Arm. In practice, this may not >> happen but it sounds better to sanity check that the BAR contains >> "valid" I/O range. >> >> On x86, this is implemented by checking the region is not described is >> in the e820. IIUC, on Arm, the BARs have to be positioned in pre-defined >> ranges. So I think it would be possible to implement is_memory_hole() by >> going through the list of hostbridges and check the ranges. >> >> But first, I'd like to confirm my understanding with Rahul, and others. > > > May I please ask about your opinion on that? I agree with Julien we can implement the something similar to is_memory_hole() for ARM that will check that the bar is within the bridge ranges. If you are okay you can discard this patch in next version of the series and I will push the patch for review. Regards, Rahul
On 03.08.22 12:29, Rahul Singh wrote: > Hi Oleksandr, Hello Rahul > >> On 29 Jul 2022, at 5:28 pm, Oleksandr <olekstysh@gmail.com> wrote: >> >> >> Hello Rahul >> >> >> On 19.07.22 20:42, Oleksandr Tyshchenko wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> Add a stub for is_memory_hole which is required for PCI passthrough >>> on Arm. >>> >>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> --- >>> OT: It looks like the discussion got stuck. As I understand this >>> patch is not immediately needed in the context of current series >>> as PCI passthrough is not enabled on Arm at the moment. So the patch >>> could be added later on, but it is needed to allow PCI passthrough >>> to be built on Arm for those who want to test it. >>> >>> Copy here some context provided by Julien: >>> >>> Here a summary of the discussion (+ some my follow-up thoughts): >>> >>> is_memory_hole() was recently introduced on x86 (see commit 75cc460a1b8c >>> "xen/pci: detect when BARs are not suitably positioned") to check >>> whether the BAR are positioned outside of a valid memory range. This was >>> introduced to work-around quirky firmware. >>> >>> In theory, this could also happen on Arm. In practice, this may not >>> happen but it sounds better to sanity check that the BAR contains >>> "valid" I/O range. >>> >>> On x86, this is implemented by checking the region is not described is >>> in the e820. IIUC, on Arm, the BARs have to be positioned in pre-defined >>> ranges. So I think it would be possible to implement is_memory_hole() by >>> going through the list of hostbridges and check the ranges. >>> >>> But first, I'd like to confirm my understanding with Rahul, and others. >> >> May I please ask about your opinion on that? > I agree with Julien we can implement the something similar to is_memory_hole() for ARM > that will check that the bar is within the bridge ranges. > > If you are okay you can discard this patch in next version of the series and I will push the patch > for review. Perfect! Thank you, that would be much appreciated! > > Regards, > Rahul
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 009b8cd9ef..bb34b97eb5 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1708,6 +1708,12 @@ unsigned long get_upper_mfn_bound(void) return max_page - 1; } +bool is_memory_hole(mfn_t start, mfn_t end) +{ + /* TODO: this needs to be properly implemented. */ + return true; +} + /* * Local variables: * mode: C