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

[Julien Grall <julien@xxxxxxx>]

On 28/01/2021 14:52, Oleksandr wrote:
> On 28.01.21 16:41, Julien Grall wrote:
> > On 28/01/2021 14:29, Oleksandr wrote:
> > > 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),
> > I looked at the header asm-x86/hvm/domain.h after applying patch #4, 
> > there is nothing requiring DMOP from there.
> > I tried to build it with this series applied up to patch #4 + my diff. 
> > It does build without any issue.
> Hmm, interesting. I might miss something, but I got an build issue if I 
> split these changes with patch #4 and build the series up to this patch:
I think I looked and tried with the wrong commit. :( This was moved in 
patch #7 (xen/ioreq: Make x86's hvm_ioreq_(page/vcpu/server) structs 
common).
Sorry for the confusion.

Cheers,