Re: [PATCH] x86/svm: Provide EXITINFO decodes for IO intercetps

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.03.2023 11:27, Andrew Cooper wrote:
> On 16/03/2023 10:19 am, Jan Beulich wrote:
>> On 15.03.2023 23:10, Andrew Cooper wrote:
>>> This removes raw number manipulation, and makes the logic easier to follow.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Thanks.
> 
> Although looking through the emails on list, apparently the vmx side of
> this got missed.

I was wondering, but then expected it would be a separate patch at some
other time.

>>> --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>>> +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>>> @@ -436,6 +436,20 @@ struct vmcb_struct {
>>>              uint64_t exitinfo2; /* offset 0x80 */
>>>          };
>>>          union {
>>> +            struct {
>>> +                bool     in:1;
>>> +                bool     :1;
>>> +                bool     str:1;
>>> +                bool     /* rep */:1;
>>> +                uint16_t bytes:3;
>>> +                uint16_t /* asz */:3;
>>> +                uint16_t /* seg */:3;
>> Is there a particular reason you comment out some of the field names? I
>> can see that "asz" might be a little odd to use, but both "rep" and "seg"
>> are imo fine to have a name even if currently there's nothing accessing
>> these fields.
> 
> There's not currently used, and in particular asz looks hard to use as
> it doesn't appear to translate nicely.  Also, I don't that seg matches
> Xen's x86_segment_* encoding.
> 
> I can uncomment them if you'd prefer, but I thought this was marginally
> safer.  I suppose it doesn't really matter.

I would appreciate if you uncommented rep and seg; leaving asz commented
is ...

> As for asz, I previously had osz to match before renaming to bytes. 
> Suggestions welcome.

... perhaps better, as the name would be misleading, and half_asz isn't
very nice either.

Jan