Re: [PATCH V2] xen/arm: dm: Drop XEN_DMOP_get_ioreq_server_info from supported

[Julien Grall <julien@xxxxxxx>]

Hi Oleksandr,

On 02/09/2025 15:00, Oleksandr Tyshchenko wrote:
> On 02.09.25 15:19, Julien Grall wrote:
>
> Hello Julien
>
> > On 02/09/2025 13:10, Orzel, Michal wrote:
> > > On 02/09/2025 13:54, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > On 02/09/2025 11:18, Orzel, Michal wrote:
> > > > > On 02/09/2025 11:49, Oleksandr Tyshchenko wrote:
> > > > > > The said sub-op is not supported on Arm, since it:
> > > > > >     - does not support the buffered emulation (so bufioreq_port/
> > > > > > bufioreq_gfn
> > > > > >       cannot be returned), please refer to ioreq_server_create()
> > > > > >     - does not support "legacy" mechanism of mapping IOREQ Server
> > > > > >       magic pages (so ioreq_gfn/bufioreq_gfn cannot be returned),
> > > > > > please
> > > > > >       refer to arch_ioreq_server_map_pages(). On Arm, only the Acquire
> > > > > >       Resource infrastructure is used to query and map the IOREQ
> > > > > > Server pages.
> > > > > >
> > > > > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > > > Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
> > > > >
> > > > > Could we perhaps add a Fixes tag here pointing to the commit
> > > > > introducing these
> > > > > DM ops and thus add this patch for this release? Not sure what
> > > > > others think.
> > > > Fixes usually implies a bug and I don't see what bug we are solving. In
> > > > fact, I don't understand why we are trying to remove the subop...
> > > Hmm, the issue is that the subop that is not supported at the moment
> > > is listed
> > > as supported in the public header.
> > [...]
> >
> > > As for the code, from safety perspective if this subop is listed
> > > explicilty in Arm's
> > > dm.c, we would need to write a separate test case and test to cover it
> > > that at
> > > the end, still returns -EOPNOTSUPP.
> > Why do you think it is not supported? AFAICT, it is still possible to
> > pass XEN_DMOP_nognfs to figure out whwether bufioreq is currently
> > available. The error code would be 0 not -EOPNOTSUPP.
>
> Yes, by passing XEN_DMOP_no_gfns we will bypass
> arch_ioreq_server_map_pages() call, and yes, ioreq_server_get_info()
> will return 0 in that case.
>
> But, "bufioreq_port" field in struct xen_dm_op_get_ioreq_server_info
> (out param) won't be really updated (since the IOREQ Server was created
> with HVM_IOREQSRV_BUFIOREQ_OFF before that).
Sure. But this would be the same behavior on x86, right? If so...

> So, "at the moment", XEN_DMOP_get_ioreq_server_info sub-op is
> non-functional/useless on Arm ("unsupported" is probably not a precise
> word in that particular case), this is my understanding (which might be
> wrong). That is why I have sent a patch to remove the mention from the
> public header.
... I still don't understand why we are just trying to sweep the problem 
under the rug on Arm.
That's assuming there is one. If there is a problem, then we should fix 
it properly for all architectures. If there is no problem, then this 
patch should not exists.
Cheers,

--
Julien Grall