Re: [PATCH v3] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 09.10.17 at 13:19, <ppircalabu@bitdefender.com> wrote:
> On Lu, 2017-10-09 at 04:36 -0600, Jan Beulich wrote:
>> > > > On 09.10.17 at 12:10, <ppircalabu@bitdefender.com> wrote:
>> > On Vi, 2017-10-06 at 09:34 -0600, Jan Beulich wrote:
>> > > > > > On 05.10.17 at 17:42, <ppircalabu@bitdefender.com> wrote:
>> > > > --- a/xen/tools/compat-build-header.py
>> > > > +++ b/xen/tools/compat-build-header.py
>> > > > @@ -16,6 +16,7 @@ pats = [
>> > > >   [ r"(8|16|32|64)_compat_t([^\w]|$)", r"\1_t\2" ],
>> > > >   [ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)",
>> > > > r"\1compat_\2_t\3"
>> > > > ],
>> > > >   [ r"(^|[^\w])XEN_?", r"\1COMPAT_" ],
>> > > > + [ r"(^|[^\w])HVMMEM_?", r"\1COMPAT_HVMMEM_" ],
>> > > What is this needed for? I can't find any instance of HVMMEM_*
>> > > elsewhere in the patch. As you can see, so far there are only
>> > > pretty generic tokens being replaced here.
>> > > 
>> > Without this, both hvmmem_type_t and hvmmem_type_compat_t enums
>> > will
>> > define the same values (e.g.  HVMMEM_ram_rw,) resulting in a
>> > compile
>> > error. By adding this translation we will have a COMPAT_HVMMEM
>> > value
>> > for each HVMMEM_ value defined in public/hvm/hvm_op.h) (e.g.
>> > HVMMEM_ram_rw -> COMPAT_HVMMEM_ram_rw)
>> That's ugly. I realize that we shouldn't even attempt to translate
>> enumerations (or to be fully precise, there shouldn't be any
>> enumerations in the public interface in the first place), as the
>> enumerator values ought to remain consistent between native
>> and compat uses. Hence we could either convert the enum to a
>> set of #define-s, or we would need a mechanism to exclude
>> parts of a header from the compat conversion.
>> 
>> In the end the problem here is because of the enumerators,
>> other than xenmem_access_t's, aren't properly prefixed with
>> XEN or XEN_ (or else the script would already handle them fine
>> afaict). So another variant of addressing this would be to
>> deprecate (but not remove) the current names, introducing
>> properly named ones for __XEN_INTERFACE_VERSION__ >=
>> 0x00040a00.
>> 
> Unfortunately the enum is referenced also in other functions
> (e.g. xendevicemodel_set_mem_type) so replacing it with #defines would
> be more difficult.
> When generating the compat headers,-DXEN_GENERATING_COMPAT_HEADERS is
> defined (xen/include/Makefile). I can guard hvmmem_type_t with an
> #ifndef XEN_GENERATING_COMPAT_HEADERS so the enum is not processed by
> the compat-build-header.py script. (in my opinion this is the minimum-
> impact approach)
> Do you agree with this?

I'm not entirely happy about that approach, but it'll do for the
moment.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel