Re: [PATCH RFC 04/10] domain: update GADDR based runstate guest area

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.12.2022 13:26, Julien Grall wrote:
> On 19/10/2022 08:41, Jan Beulich wrote:
>> Before adding a new vCPU operation to register the runstate area by
>> guest-physical address, add code to actually keep such areas up-to-date.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> RFC: Pages aren't marked dirty when written to (matching the handling of
>>       space mapped by map_vcpu_info() afaict), on the basis that the
>>       registrations are lost anyway across migration. 
> 
> So I agree for the existing migration. But I wonder whether we would 
> need to dirty those pages if we live-migrated a guest without its help 
> (IOW the registrations would still be present).

Even then I'd expect the area to be re-populated at the target, so the
page contents would need moving over (perhaps multiple times) only if
any other parts of such a page were written to.

> Anyway, nothing to worry about yet as this is not supported upstream.

I assume you've taken note of this for the transparent migration work.
One question after all is how you'd make handling of the area at the
new target transparent, i.e. without any anomalies in the values the
guest gets to see. It may very well be that every such area needs
special treatment in the course of migrating, such that the most up-
to-date values are reported as part of the migration stream, but
separate from all the pages' contents.

>> Plus the contents
>>       of the areas in question have to be deemed volatile in the first
>>       place (so saving a "most recent" value is pretty meaningless even
>>       for e.g. snapshotting).
>>
>> RFC: Can we perhaps avoid the VM-assist conditionals, assuming the more
>>       modern behavior to apply uniformly for gaddr-based registrations?
> 
> It is not clear why someone would want to use the old behavior with the 
> new gaddr-based registrations. So I would say yes.

Okay, will do.

>> RFC: HVM guests (on x86) can change bitness and hence layout (and size!
>>       and alignment) of the runstate area. I don't think it is an option
>>       to require 32-bit code to pass a range such that even the 64-bit
>>       layout wouldn't cross a page boundary (and be suitably aligned). I
>>       also don't see any other good solution, so for now a crude approach
>>       with an extra boolean is used (using has_32bit_shinfo() isn't race
>>       free and could hence lead to overrunning the mapped space).
> 
> I think the extra check for 32-bit code to pass the check for 64-bit 
> layout would be better.

I'm afraid I can't derive from your reply what it is you actually want.

Jan