On 21.02.2024 10:34, Julien Grall wrote: > 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?
On the grounds of the argument I gave before: Consistency with other more or less similar operations, where length 0 simply means "no-op", up to and including "no errors from arguments specifying the address(es) to operate on". Jan
