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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 20/02/2024 12:25, Jan Beulich wrote:
> On 20.02.2024 12:52, Julien Grall wrote:
> > 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).
> I don't view this as primarily an optimization; I'm in particular after
> not raising errors for cases where there is no error to be raised.
> Hence the comparison to memcpy(), which you can pass "bogus" pointers
> so long as you pass zero size.
The part I am missing is why this approach is better than what we have. 
So far what you described is just a matter of taste.
To give a concrete example, if tomorrow a contributor decides to send a 
patch undoing what you did (IOW enforcing the check for zero-length or 
replace | with two branches), then on what grounds I will be able to 
refuse their patch?
Cheers,

--
Julien Grall