Re: [Xen-devel] [PATCH] x86: move vgc_flags to struct pv_vcpu

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.01.2020 11:56, Julien Grall wrote:
> Hi,
> 
> On 27/12/2019 12:14, Jan Beulich wrote:
>> On 27.12.2019 12:27, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@xxxxxxxx> wrote:
>>>
>>>> On 23.12.2019 18:33, Julien Grall wrote:
>>>>> Hi Jan,
>>>>>
>>>>> On 20/12/2019 14:55, Jan Beulich wrote:
>>>>>> There's been effectively no use of the field for HVM.
>>>>>>
>>>>>> Also shrink the field to unsigned int, even if this doesn't immediately
>>>>>> yield any space benefit for the structure itself. The resulting 32-bit
>>>>>> padding slot can eventually be used for some other field. The change in
>>>>>> size makes accesses slightly more efficient though, as no REX.W prefix
>>>>>> is going to be needed anymore on the respective insns.
>>>>>>
>>>>>> Mirror the HVM side change here (dropping of setting the field to
>>>>>> VGCF_online) also to Arm, on the assumption that it was cloned like
>>>>>> this originally. VGCF_online really should simply and consistently be
>>>>>> the guest view of the inverse of VPF_down, and hence needs representing
>>>>>> only in the get/set vCPU context interfaces.
>>>>>
>>>>> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
>>>>> changes below will clearly break bring-up of secondary vCPUs on Arm.
>>>>>
>>>>> This is because arch_set_guest_info() will rely on this flag to
>>>>> clear/set VPF_down in the pause flags.
>>>>>
>>>>> So I think the line in arm/vpsci.c should be left alone.
>>>>
>>>> Oh, I see - I didn't notice this (ab)use despite ...
>>>>
>>>
>>> Out of Interest, why do you think it is an abuse here and not in the
>>> toolstack?
>>>
>>> How do you suggest to improve it? I can add it in my improvement list for
>>> Arm.
>>
>> Oh, "abuse" was about the arch_set_guest_info() use, not the use of
>> the flag by the tool stack.
> 
> I may have read incorrectly your e-mail, although I think my questions 
> about why this is an abuse and how do you suggest to improve are still 
> relevant.

arch_set_info_guest() is intended to be used for exactly one purpose
- vCPU context initialization via hypercall. With this, and _without_
me knowing anything about PSCI, it _looks_ to me to be an abuse. I'd
expect there to be something in x86 that could be used for
comparison, and whatever that is - it doesn't need a similar extra
use of arch_set_info_guest(). (As a result, I don't see how I could
reasonably give a concrete suggestion towards improvement. In fact I
may be entirely wrong with my feeling of this being an abuse in the
first place.)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel