On 02/09/2024 4:09 pm, Jan Beulich wrote:
> On 02.09.2024 16:13, Andrew Cooper wrote:
>> On 02/09/2024 1:28 pm, Jan Beulich wrote:
>>> Taking a fault on a non-byte-granular insn means that the "number of
>>> bytes not handled" return value would need extra care in calculating, if
>>> we want callers to be able to derive e.g. exception context (to be
>>> injected to the guest) - CR2 for #PF in particular - from the value. To
>>> simplify things rather than complicating them, reduce inline assembly to
>>> just byte-granular string insns. On recent CPUs that's also supposed to
>>> be more efficient anyway.
>>>
>>> For singular element accessors, however, alignment checks are added,
>>> hence slightly complicating the code. Misaligned (user) buffer accesses
>>> will now be forwarded to copy_{from,to}_guest_ll().
>>>
>>> Naturally copy_{from,to}_unsafe_ll() accessors end up being adjusted the
>>> same way, as they're produced by mere re-processing of the same code.
>>> Otoh copy_{from,to}_unsafe() aren't similarly adjusted, but have their
>>> comments made match reality; down the road we may want to change their
>>> return types, e.g. to bool.
>>>
>>> Fixes: 76974398a63c ("Added user-memory accessing functionality for x86_64")
>>> Fixes: 7b8c36701d26 ("Introduce clear_user and clear_guest")
>>> Reported-by: Andrew Cooper <[email protected]>
>>> Signed-off-by: Jan Beulich <[email protected]>
>> This is definitely simpler, however the code gen less so.
>>
>> add/remove: 0/0 grow/shrink: 42/9 up/down: 2759/-178 (2581)
> Not nice, but entirely expected.Yes. Having considered this for a while, the usual rule prevails; get it correct first, fast second. So, lets go with it like this. I already want to get rid of .fixup for backtrace reasons, so don't want to go expanding our use of it. I'm also not thrilled with the idea of doing a second access. At the time any fault has occurred, we're delivering an error of some form, and finding unexpected success the second time around is about the worst available outcome. As to your comment about to-guest, that does matter for correct %cr2 on an emulated store. The INS emulation tries precisely this. I've got some ad-hoc XTF tests which I'll run this patch over, but I'm expecting to R-by/T-by in this form. ~Andrew
