Re: [PATCH V2 02/23] xen/ioreq: Make x86's IOREQ feature common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 20.10.20 10:57, Paul Durrant wrote:

Hi Paul

Sorry for the late response.

> > -----Original Message-----
> > From: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>
> > Sent: 15 October 2020 17:44
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>; Andrew Cooper 
> > <andrew.cooper3@xxxxxxxxxx>;
> > George Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson <iwj@xxxxxxxxxxxxxx>; Jan 
> > Beulich
> > <jbeulich@xxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini 
> > <sstabellini@xxxxxxxxxx>; Wei
> > Liu <wl@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>; Paul Durrant 
> > <paul@xxxxxxx>; Tim Deegan
> > <tim@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
> > Subject: [PATCH V2 02/23] xen/ioreq: Make x86's IOREQ feature common
> >
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> >
> > As a lot of x86 code can be re-used on Arm later on, this patch
> > moves previously prepared x86/hvm/ioreq.c to the common code.
> >
> > The common IOREQ feature is supposed to be built with IOREQ_SERVER
> > option enabled, which is selected for x86's config HVM for now.
> >
> > In order to avoid having a gigantic patch here, the subsequent
> > patches will update remaining bits in the common code step by step:
> > - Make IOREQ related structs/materials common
> > - Drop the "hvm" prefixes and infixes
> FWIW you could tackle the naming changes in patch #1.
Unfortunately, there are a lot of places that need touching (in order to 
replace/drop hvm prefixes), so I decided to keep that in a separate patch.
From the review comments on previous series I got that renaming could 
be done before or after the move, but within
a single series, so I chose the latter.

> The 'legacy' mechanism of mapping magic pages for ioreq servers should remain 
> x86 specific I think that aspect of the code needs to remain behind and not get 
> moved into common code. You could do that in arch specific calls in 
> hvm_ioreq_server_enable/disable() and hvm_get_ioreq_server_info().
Well, if legacy mechanism is not going to be used for other arch and 
should remain x86 specific, I will try to investigate what should be 
left in x86 code and rework the series.
As a side note, I am afraid, we won't get a 100% code movement (which I 
managed to achieve here) for the next version of this patch as we need 
arch/x86/hvm/ioreq.c.

--
Regards,

Oleksandr Tyshchenko