Re: [PATCH 2/2] almost fully ignore zero-size flush requests

[Julien Grall <julien@xxxxxxx>]

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