Re: [PATCH V5 14/22] arm/ioreq: Introduce arch specific bits for IOREQ/DM features

[Oleksandr <olekstysh@gmail.com>]

Hi Julien


On 28.01.21 15:39, Oleksandr wrote:
>
> On 28.01.21 13:33, Oleksandr wrote:
>
> Hi Julien
>
>>
>> On 28.01.21 11:40, Julien Grall wrote:
>>
>> Hi Julien
>>
>>> Hi Oleksandr,
>>>
>>> On 27/01/2021 19:20, Oleksandr wrote:
>>>  >  >>> So I think we may be able to drop the include from 
>>> asm/hvm/domain.h
>>>> (this would avoid to include it everywhere...).
>>>>
>>>> I have tried that, but other CUs use definitions from 
>>>> public/hvm/dm_op.h, for example:
>>>>
>>>> p2m-pt.c: In function 'p2m_type_to_flags':
>>>> p2m-pt.c:87:33: error: 'XEN_DMOP_IOREQ_MEM_ACCESS_WRITE' undeclared 
>>>> (first use in this function)
>>>>           if ( p2m->ioreq.flags & XEN_DMOP_IOREQ_MEM_ACCESS_WRITE )
>>>>                                   ^
>>>> So, I would prefer to leave it as is, please let me know if you 
>>>> think otherwise.
>>>
>>> AFAICT, there is only 2 places (p2m-pt.c and p2m-ept.c) that 
>>> requires <public/hvm/dm_op.h> but doesn't directly include it. 
>>> Folding the diff below in patch #4 should do the job:
>>
>> ok, will do
>
>
> Just to clarify, you mentioned about patch #4, but shouldn't we make 
> these changes in patch #9 which actually tries to sort dm related stuff?

or a least in patch #8 which moves the stuff from asm-x86/hvm/domain.h 
to xen/ioreq.h (including the user of XEN_DMOP_IO_RANGE_PCI),

what do you think?


As for proposed changes, can confirm they work.


>
>
>>
>>
>>>
>>> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
>>> index 975ab403f235..23d411f01d2f 100644
>>> --- a/xen/arch/x86/mm/p2m-ept.c
>>> +++ b/xen/arch/x86/mm/p2m-ept.c
>>> @@ -17,6 +17,7 @@
>>>
>>>  #include <xen/domain_page.h>
>>>  #include <xen/sched.h>
>>> +#include <public/hvm/dm_op.h>
>>>  #include <asm/altp2m.h>
>>>  #include <asm/current.h>
>>>  #include <asm/paging.h>
>>> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
>>> index c43d5d0413a1..f2afcf49a368 100644
>>> --- a/xen/arch/x86/mm/p2m-pt.c
>>> +++ b/xen/arch/x86/mm/p2m-pt.c
>>> @@ -27,6 +27,7 @@
>>>  #include <xen/vm_event.h>
>>>  #include <xen/event.h>
>>>  #include <xen/trace.h>
>>> +#include <public/hvm/dm_op.h>
>>>  #include <public/vm_event.h>
>>>  #include <asm/altp2m.h>
>>>  #include <asm/domain.h>
>>> diff --git a/xen/include/asm-x86/hvm/domain.h 
>>> b/xen/include/asm-x86/hvm/domain.h
>>> index 3b36c2f41fa1..f26c1a2e2d5f 100644
>>> --- a/xen/include/asm-x86/hvm/domain.h
>>> +++ b/xen/include/asm-x86/hvm/domain.h
>>> @@ -28,8 +28,6 @@
>>>  #include <asm/hvm/vmx/vmcs.h>
>>>  #include <asm/hvm/svm/vmcb.h>
>>>
>>> -#include <public/hvm/dm_op.h>
>>> -
>>>  #ifdef CONFIG_MEM_SHARING
>>>  struct mem_sharing_domain
>>>  {
>>>
>>> You would then need to move the include of <public/hvm/dm_op.h> in 
>>> <xen/dm.h> from this patch to patch #9.
>>
>> yes, sure
>>
>>
-- 
Regards,

Oleksandr Tyshchenko