Re: [PATCH for-4.17] x86/hvm: Revert per-domain APIC acceleration support

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Nov 15, 2022 at 12:35:39AM +0000, Andrew Cooper wrote:
> I was really hoping to avoid this, but its now too late in the 4.17 freeze and
> we still don't have working fixes.

The fix I proposed still has some comments that have been unanswered,
but at any rate thit is now far too late to try to get this fixed.

FWIW that same fix was also posted to the security list way before
hitting xen-devel.

> The in-Xen calculations for assistance capabilities are buggy.  For the
> avoidance of doubt, the original intention was to be able to control every
> aspect of a APIC acceleration so we could comprehensively test Xen's support,
> as it has proved to be buggy time and time again.
> 
> Even after a protracted discussion on what the new API ought to mean, attempts
> to apply it to the existing logic have been unsuccessful, proving that the
> API/ABI is too complicated for most people to reason about.

Are you referring to the VMX hardware interface to setup the related
APIC assistance flags, or the hypervisor interface to control those
features?

> This reverts most of:
>   2ce11ce249a3981bac50914c6a90f681ad7a4222
>   6b2b9b3405092c3ad38d7342988a584b8efa674c
> 
> leaving in place the non-APIC specific changes (minimal as they are).
> 
> This takes us back to the behaviour of Xen 4.16 where APIC acceleration is
> configured on a per system basis.
> 
> This work will be revisited in due course.

I certainly regret having been involved in attempting to fix this, and
I have to admit I still don't understand what is broken with the
current API/ABI.

Do we want a flag to control the setting of the APIC register
virtualization feature?

Is the naming for the flag that we expose incorrect?

Is the field where the flag gets set incorrect?

There isn't that much to the current interface.

In the previous reply to the fix however I got the (maybe incorrect)
impression that current bugs in the implementation are used as a way
to justify why the interface is broken, and that is not accurate.  The
interface and the implementation are two different things, and bugs
in the implementation shouldn't automatically invalidate the
interface without further reasoning.

Also, for better or worse, the current domctl interface where all this
is implemented is unstable, and hence we are allowed to make further
changes as we implement more related features.

> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I'm not going to oppose to the revert, but as said above it is still
not clean to me what is broken in the current approach apart from
implementation bugs, hence it's unlikely for me to revisit this work,
at least not until such uncertainty is solved.

Regards, Roger.