Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Nov 11, 2020 at 01:10:01PM +0000, Oleksandr Andrushchenko wrote:
> 
> On 11/11/20 2:31 PM, Roger Pau Monné wrote:
> > On Mon, Nov 09, 2020 at 02:50:22PM +0200, Oleksandr Andrushchenko wrote:
> >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >>
> >> According to https://wiki.xenproject.org/wiki/Linux_PVH:
> >>
> >> Items not supported by PVH
> >>   - PCI pass through (as of Xen 4.10)
> >>
> >> Allow running PCI remove code on ARM and do not assert for PVH domains.
> >>
> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >> ---
> >>   tools/libxl/Makefile    | 4 ++++
> >>   tools/libxl/libxl_pci.c | 4 +++-
> >>   2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> >> index 241da7fff6f4..f3806aafcb4e 100644
> >> --- a/tools/libxl/Makefile
> >> +++ b/tools/libxl/Makefile
> >> @@ -130,6 +130,10 @@ endif
> >>   
> >>   LIBXL_LIBS += -lyajl
> >>   
> >> +ifeq ($(CONFIG_ARM),y)
> >> +CFALGS += -DCONFIG_ARM
> >> +endif
> >> +
> >>   LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
> >>                    libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
> >>                    libxl_internal.o libxl_utils.o libxl_uuid.o \
> >> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> >> index bc5843b13701..b93cf976642b 100644
> >> --- a/tools/libxl/libxl_pci.c
> >> +++ b/tools/libxl/libxl_pci.c
> >> @@ -1915,8 +1915,10 @@ static void do_pci_remove(libxl__egc *egc, uint32_t 
> >> domid,
> >>               goto out_fail;
> >>           }
> >>       } else {
> >> +        /* PCI passthrough can also run on ARM PVH */
> >> +#ifndef CONFIG_ARM
> >>           assert(type == LIBXL_DOMAIN_TYPE_PV);
> >> -
> >> +#endif
> > I would just remove the assert now if this is to be used by Arm and
> > you don't need to fork the file for Arm.
> 
> Sounds good, I will drop then
> 
> But what would be the right explanation then? I mean why there was an ASSERT
> 
> and now it is safe (for x86) to remove that?

An assert is just a safe belt, the expectation is that it's never hit
by actual code. Given that this path will now also be used by PVH
(even if only on Arm) I don't see the point in keeping the assert, and
making it conditional to != Arm seems worse than just dropping it.

Thanks, Roger.