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 <[email protected]> >>>> Reviewed-by: Michal Orzel <[email protected]> >>>> >>>> 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). 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 think if we mistakenly mention sth as> supported in e.g. SUPPORT.md > we would have no issues adding a Fixes tag. There > > are many cases where Fixes was used just to change something in a > comment, so > > I'm having a hard time reasoning about when it's appropriate to use it. > I think what we would want is "Amends". This is currently proposed by [1]. Good point. > > [1] https://eur01.safelinks.protection.outlook.com/? > url=https%3A%2F%2Flore.kernel.org%2Fxen-devel%2Fe7c99116-f6a9-43e1-80ae- > b3a4d44857b7%40amd.com%2FT%2F%23t&data=05%7C02%7COleksandr_Tyshchenko%40epam.com%7C27024902b14c42b7eaf608ddea1b0173%7Cb41b72d04e9f4c268a69f949f367c91d%7C1%7C0%7C638924123934835957%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=7o1CpNkXPxHQqqnWBPEUy1Q1%2BjL%2FM7VmTrMT7fOu4Lw%3D&reserved=0 > >> >> ~Michal >> >
