1. Patchew
  2. Xen
  3. [PATCH 00/10] [RFC] ARM PCI passthrough configuration and vPCI
  4. View patch

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

Oleksandr Andrushchenko posted 10 patches 4 years ago
[PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Oleksandr Andrushchenko 4 years ago 
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

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@epam.com>
---
 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
         char *sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
                                      pcidev->bus, pcidev->dev, pcidev->func);
         FILE *f = fopen(sysfs_path, "r");
-- 
2.17.1
Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Roger Pau Monné 4 years ago 
On Mon, Nov 09, 2020 at 02:50:22PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 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@epam.com>
> ---
>  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.

Roger.
Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Oleksandr Andrushchenko 4 years ago 
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@epam.com>
>>
>> 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@epam.com>
>> ---
>>   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?

>
> Roger.

Thank you,

Oleksandr
Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Roger Pau Monné 4 years ago 
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@epam.com>
> >>
> >> 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@epam.com>
> >> ---
> >>   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.
Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Oleksandr Andrushchenko 4 years ago 
On 11/11/20 3:55 PM, Roger Pau Monné wrote:
> 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@epam.com>
>>>>
>>>> According to https://urldefense.com/v3/__https://wiki.xenproject.org/wiki/Linux_PVH__;!!GF_29dbcQIUBPA!nEHd6eivmqtdJxtrhO-3x2Mz9F50JsKUoV7WTEJd_D1N01DrBOJXzGW1QAqwshZ9AMxywbUhOA$ [wiki[.]xenproject[.]org]:
>>>>
>>>> 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@epam.com>
>>>> ---
>>>>    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.

Ok, so I can write in the patch description something like:

"this path is now used by PVH, so the assert is no longer valid"

Does it sound ok?

> Thanks, Roger.

Thank you,

Oleksandr
Re: [SUSPECTED SPAM][PATCH 01/10] pci/pvh: Allow PCI toolstack code run with PVH domains on ARM
Posted by Roger Pau Monné 4 years ago 
On Wed, Nov 11, 2020 at 02:12:56PM +0000, Oleksandr Andrushchenko wrote:
> 
> On 11/11/20 3:55 PM, Roger Pau Monné wrote:
> > 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@epam.com>
> >>>>
> >>>> According to https://urldefense.com/v3/__https://wiki.xenproject.org/wiki/Linux_PVH__;!!GF_29dbcQIUBPA!nEHd6eivmqtdJxtrhO-3x2Mz9F50JsKUoV7WTEJd_D1N01DrBOJXzGW1QAqwshZ9AMxywbUhOA$ [wiki[.]xenproject[.]org]:
> >>>>
> >>>> 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@epam.com>
> >>>> ---
> >>>>    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.
> 
> Ok, so I can write in the patch description something like:
> 
> "this path is now used by PVH, so the assert is no longer valid"
> 
> Does it sound ok?

LGTM.

Roger.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.