Re: shrink ci_ilevel from 32 to 8 bits ?

[Andrew Doran <ad%netbsd.org@localhost>]

Hi Manuel,

On Sat, Apr 11, 2020 at 10:25:48PM +0200, Manuel Bouyer wrote:

> Hello,
> at some point, the x86's cpu_info was changed to pack the istate in a
> structure:
>         struct {
>                 uint32_t        ipending;
>                 int             ilevel;
>                 uint32_t        imasked;
>         } ci_istate __aligned(8);
> #define ci_ipending     ci_istate.ipending
> #define ci_ilevel       ci_istate.ilevel
> #define ci_imasked      ci_istate.imasked
> 
> The point was to be able to change ipending and ilevel together
> atomically, with cmpxchg8b. This is used in spllower() and mutex_spin_exit().
> This avoid using sti/cli which is more expansive.
> 
> For Xen, a second interrupt pending integer exists, ci_xpending. It is
> separate because for PVHVM, and probably PVH, we need both.
> we can't use the native ipending mechanism because 32 events are really not
> enough, especially for dom0. It is not a pending interrupt maks, but
> a pending IPL mask.
> 
> With now, amd64's mutex_spin_exit checks the native pending interrupts
> and resets ilevel using cmpxchg8b, then checks xpending. This is racy.
> Also, as the goal is to get PVHVM support back in GENERIC, we can't really
> go back to sti/cli.
> 
> So I'ld like to change ci_istate to
>         struct {
>                 uint32_t        ipending;
>                 int8_t          ilevel;
>                 uint8_t         xpending;
>                 int16_t         ipad; /* free bits for futur use */
>                 uint32_t        imasked;
>         } ci_istate __aligned(8);
> #define ci_ipending     ci_istate.ipending
> #define ci_ipendin     ci_istate.xpending
> #define ci_ilevel       ci_istate.ilevel
> #define ci_imasked      ci_istate.imasked
> 
> and change the assembly code to access ilevel as byte instead of long
> (e.g. with movsbl instead of movl).
>
> Does anyone see a problem with this approach ?

That sounds reasonable to me.

Andrew