On 27.09.2022 09:02, Juergen Gross wrote:
> On 26.09.22 17:52, Jan Beulich wrote:
>> On 26.09.2022 10:33, Juergen Gross wrote:
>>> On 26.09.22 09:53, Jan Beulich wrote:
>>>> On 23.09.2022 10:20, Juergen Gross wrote:
>>>>> My favorite solution would be some kind of buffer address qualifier for
>>>>> each
>>>>> buffer (e.g. virtual, physical, SG-list, maybe nested SG-list). So the new
>>>>> hypercalls would not mean "physical buffer addresses", but "qualified
>>>>> buffer
>>>>> addresses". By requiring a minimum of 4-byte alignment for each buffer
>>>>> (can we
>>>>> do that, at least for the new hypercalls?) this would leave the 2 lowest
>>>>> bits
>>>>> of a buffer address for the new qualifier. If by any means an unaligned
>>>>> buffer
>>>>> is needed sometimes, it could still be achieved via a single-entry
>>>>> SG-list.
>>>>
>>>> While this might be an option, I'm not sure I'd be really happy with such
>>>> re-use of the low address bits, nor with the implied further restriction
>>>> on buffer alignment (most struct-s we use are 4-byte aligned at least,
>>>> but I don't think it's all of them, plus we also have guest handles to
>>>> e.g. arrays of char).
>>>
>>> The unaligned cases could be handled dynamically via the single-entry
>>> SG-list.
>>
>> Can they? The first example you gave, the bitmap passed to collect the
>> output of XEN_DOMCTL_SHADOW_OP_{CLEAN,PEEK}, comes as a handle-of-uint8,
>> i.e. generally large but not necessarily aligned (even if in practice
>> the caller likely will pass a page aligned buffer of multiple pages in
>> size). If we introduced physical-address bases replacement sub-ops, I
>> think we would make the buffer described by an array of GFNs, not even
>> allowing sub-page alignment or size.
>
> In case the buffer is crossing page boundaries, the SG-list would need to
> have more than one entry, of course (assuming the SG-list variant is chosen).
Of course, but that wasn't the point I was trying to get at. How would the
(generic, i.e. alignment unaware) copying logic know the low bits are not
a descriptor in this case? I'd rather not see us have e.g. both
copy_from_guest_pv() and copy_from_guest_pv_unaligned() (nor a 4th argument
to the former, to express the same).
Jan
