Re: git: 76887e84be97 - main - cpuset: increase userland maximum size to 1024

From: Mateusz Guzik <mjguzik_at_gmail.com>
Date: Tue, 09 May 2023 09:52:38 UTC
On 5/9/23, Corvin Köhne <corvink@freebsd.org> wrote:
> On Tue, 2023-05-09 at 09:43 +0200, Corvin Köhne wrote:
>> On Tue, 2023-05-09 at 08:59 +0200, Corvin Köhne wrote:
>> > On Mon, 2023-05-08 at 18:26 +0000, Ed Maste wrote:
>> > > The branch main has been updated by emaste:
>> > >
>> > > URL:
>> > > https://cgit.FreeBSD.org/src/commit/?id=76887e84be975698b14699d7d0dfb157d73e9990
>> > >
>> > > commit 76887e84be975698b14699d7d0dfb157d73e9990
>> > > Author:     Ed Maste <emaste@FreeBSD.org>
>> > > AuthorDate: 2023-05-02 20:57:12 +0000
>> > > Commit:     Ed Maste <emaste@FreeBSD.org>
>> > > CommitDate: 2023-05-08 18:25:15 +0000
>> > >
>> > >     cpuset: increase userland maximum size to 1024
>> > >
>> > >     Hardware with more than 256 CPU cores is now available and
>> > > will
>> > > become
>> > >     increasingly common.  Bump CPU_MAXSIZE (used for userland
>> > > cpuset_t
>> > >     sizing) to 1024 to define the ABI for FreeBSD 14.
>> > >
>> > >     PR:             269572, 271213 [exp-run]
>> > >     Reviewed by:    mjg, jhb
>> > >     Relnotes:       yes
>> > >     Sponsored by:   The FreeBSD Foundation
>> > >     Differential Revision: https://reviews.freebsd.org/D39941
>> > > ---
>> > >  sys/sys/_cpuset.h | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/sys/sys/_cpuset.h b/sys/sys/_cpuset.h
>> > > index b75ee595da3c..676f9855f384 100644
>> > > --- a/sys/sys/_cpuset.h
>> > > +++ b/sys/sys/_cpuset.h
>> > > @@ -40,7 +40,7 @@
>> > >  #define        CPU_SETSIZE     MAXCPU
>> > >  #endif
>> > >
>> > > -#define        CPU_MAXSIZE     256
>> > > +#define        CPU_MAXSIZE     1024
>> > >
>> > >  #ifndef        CPU_SETSIZE
>> > >  #define        CPU_SETSIZE     CPU_MAXSIZE
>> >
>> > Hi Ed,
>> >
>> > this somehow breaks bhyve on my system. It immediately exits with
>> > "vm_run error -1" on startup.
>> >
>> >
>>
>> Setting MAXCPU [1] to 1024 as well, solves the issue. Not sure if
>> it's
>> a regression of your commit or a bhyve bug.
>>
>> [1]
>> https://github.com/freebsd/freebsd-src/blob/20d8d9809a8c0a3ddd1d0156e77ec53a1ed95747/sys/amd64/include/param.h#L68
>>
>>
>
> vmm kernel module and bhyve userland are sharing a cpuset_t. So, this
> commit introduces a mismatch in the ioctl signature of VM_RUN. Any
> advice how to fix this properly, would be appreciated.
>
> [1]
> https://github.com/freebsd/freebsd-src/blob/c543e09f1f8ef98f201f7dd3f34ae023d61dfa83/sys/amd64/include/vmm.h#L759
>
>
>

On one hand this really should not be a part of the abi (you could
have a pointer to cpuset and a a var stating the size).

On the other hand, i suspect the least problematic long term solution
is to introduce usercpuset_t (or whatever the name) and have a
conversion func on import. This would still mean you need a kernel and
user variant of the vm_exit struct.

-- 
Mateusz Guzik <mjguzik gmail.com>