Re: [PATCH 1/2] common: map_vcpu_info() cosmetics

[Julien Grall <julien@xxxxxxx>]

On 17/07/2020 09:16, Jan Beulich wrote:
> On 16.07.2020 18:17, Julien Grall wrote:
> > On Thu, 16 Jul 2020, 17:01 Jan Beulich, <jbeulich@xxxxxxxx> wrote:
> >
> > > On 16.07.2020 16:42, Roger Pau Monné wrote:
> > > > On Thu, Jul 16, 2020 at 01:48:51PM +0200, Jan Beulich wrote:
> > > > > On 16.07.2020 13:41, Roger Pau Monné wrote:
> > > > > > On Wed, Jul 15, 2020 at 12:15:10PM +0200, Jan Beulich wrote:
> > > > > > > Use ENXIO instead of EINVAL to cover the two cases of the address not
> > > > > > > satisfying the requirements. This will make an issue here better stand
> > > > > > > out at the call site.
> > > > > > Not sure whether I would use EFAULT instead of ENXIO, as the
> > > > > > description of it is 'bad address' which seems more inline with the
> > > > > > error that we are trying to report.
> > > > > The address isn't bad in the sense of causing a fault, it's just
> > > > > that we elect to not allow it. Hence I don't think EFAULT is
> > > > > suitable. I'm open to replacement suggestions for ENXIO, though.
> > > > Well, using an address that's not properly aligned to the requirements
> > > > of an interface would cause a fault? (in this case it's a software
> > > > interface, but the concept applies equally).
> > > Not necessarily, see x86'es behavior. Also even on strict arches
> > it is typically possible to cover for the misalignment by using
> > > suitable instructions; it's still an implementation choice to not
> > > do so.
> >
> > I am not sure about your argument here... Yes it might be possible, but at
> > what cost?
> The cost is what influences the decision whether to support it. Nevertheless
> it remains an implementation decision rather than a hardware imposed
> restriction, and hence I don't consider -EFAULT suitable here.
>
> > > > Anyway, not something worth arguing about I think, so unless someone
> > > > else disagrees I'm fine with using ENXIO.
> > > Good, thanks.
> > -EFAULT can be described as "Bad address". I think it makes more sense than
> > -ENXIO here even if it may not strictly result to a fault on some arch.
> As said - I don't consider EFAULT applicable here;
AFAICT, you don't consider it because you think that using the address 
means it will always lead to a fault. However, this is just a strict 
interpretation of the error code. A less strict interpretation is it 
could be used for any address that is considered to be invalid.
-ENXIO makes less sense because the address exists. To re-use your 
argument, this is just an implementation details.
> I also consider EINVAL
> as too generic. I'll be happy to see replacement suggestions for my ENXIO
> choice, but obviously I'm not overly happy to see options re-suggested
> which I did already say I've ruled out.
I think I am allowed to express my opinion even if it means this was 
already said... However, I should have been clearer and say that I agree 
with Roger's suggestion about -EFAULT.
Cheers,

--
Julien Grall