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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

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@xxxxxxxx>
>>>>
>>>> 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@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.

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