On Tue, Mar 08, 2022 at 01:24:23PM +0100, Jan Beulich wrote:
> On 08.03.2022 12:38, Roger Pau Monné wrote:
> > On Mon, Mar 07, 2022 at 03:06:09PM +0000, Jane Malalane wrote:
> >> @@ -685,13 +687,31 @@ int arch_sanitise_domain_config(struct
> >> xen_domctl_createdomain *config)
> >> }
> >> }
> >>
> >> - if ( config->arch.misc_flags & ~XEN_X86_MSR_RELAXED )
> >> + if ( config->arch.misc_flags & ~(XEN_X86_MSR_RELAXED |
> >> + XEN_X86_ASSISTED_XAPIC |
> >> + XEN_X86_ASSISTED_X2APIC) )
> >> {
> >> dprintk(XENLOG_INFO, "Invalid arch misc flags %#x\n",
> >> config->arch.misc_flags);
> >> return -EINVAL;
> >> }
> >>
> >> + if ( (assisted_xapic || assisted_x2apic) && !hvm )
> >> + {
> >> + dprintk(XENLOG_INFO,
> >> + "Interrupt Controller Virtualization not supported for
> >> PV\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if ( (assisted_xapic && !assisted_xapic_available) ||
> >> + (assisted_x2apic && !assisted_x2apic_available) )
> >> + {
> >> + dprintk(XENLOG_INFO,
> >> + "Hardware assisted x%sAPIC requested but not available\n",
> >> + assisted_xapic && !assisted_xapic_available ? "" : "2");
> >> + return -EINVAL;
> >
> > I think for those two you could return -ENODEV if others agree.
>
> If by "two" you mean the xAPIC and x2APIC aspects here (and not e.g. this
> and the earlier if()), then I agree. I'm always in favor of using distinct
> error codes when possible and at least halfway sensible.
I would be fine by using it for the !hvm if also. IMO it makes sense
as PV doesn't have an APIC 'device' at all, so ENODEV would seem
fitting. EINVAL is also fine as the caller shouldn't even attempt that
in the first place.
So let's use it for the last if only.
Thanks, Roger.
