Hi Jan,
On 20/02/2024 08:26, Jan Beulich wrote:
On 19.02.2024 23:22, Julien Grall wrote:
Title: I would add 'gnttab:' to clarify which subsystem you are modifying.
That's how I actually have it here; it's not clear to me why I lost the
prefix when sending.
On 05/02/2024 11:03, Jan Beulich wrote:
Along the line with observations in the context of XSA-448, besides
"op" no field is relevant when the range to be flushed is empty, much
like e.g. the pointers passed to memcpy() are irrelevant (and would
never be "validated") when the passed length is zero. Split the existing
condition validating "op", "offset", and "length", leaving only the "op"
part ahead of the check for length being zero (or no flushing to be
performed).
I am probably missing something here. I understand the theory behind
reducing the number of checks when len == 0. But an OS cannot rely on it:
1) older hypervisor would still return an error if the check doesn't
pass)
Right, but that's no reason to keep the bogus earlier behavior.
Hmmm... I am not sure why you say the behavior is bogus. From the commit
message, it seems this is just an optimization that have side effect
(ignoring the other fields).
2) it does feel odd to allow "invalid" offset when len == 0 (at least.
I'm puzzled: You've given R-b for patch 1 (thanks), where exactly the
same reasoning is used, i.e. similarly referring to memcpy() to
justify the (new / supposed) behavior.
I realize it. But I viewed it slightly different as you are adding the
check. I think it is a good idea to add the check and ideally it should
be after.
Here you don't seem to add any check and only re-order it. Hence why I
treated it differently.
Cheers,
--
Julien Grall
