On Mon Nov 17, 2025 at 3:09 PM CET, Jan Beulich wrote: > On 17.11.2025 14:43, Teddy Astie wrote: >> Le 17/11/2025 à 13:46, Jan Beulich a écrit : >>> On 17.11.2025 13:35, Teddy Astie wrote: >>>> A 4K page appears to be able to hold 128 ioreq entries, which luckly >>>> matches the current vCPU limit. However, if we decide to increase the >>>> domain vCPU limit, that doesn't hold anymore and this function would now >>>> silently create a out of bounds pointer leading to confusing problems. >>>> >>>> All architectures with ioreq support don't support 128 vCPU limit for >>>> HVM guests, and have pages that are at least 4 KB large, so this case >>>> doesn't occurs in with the current limits. >>>> >>>> For the time being, make sure we can't make a Xen build that can >>>> accidentally make a out of bounds pointers here. >>>> >>>> No functional change. >>>> >>>> Reported-by: Julian Vetter <[email protected]> >>>> Signed-off-by: Teddy Astie <[email protected]> >>> >>> I was meaning to ack this, but ... >>> >>>> --- a/xen/common/ioreq.c >>>> +++ b/xen/common/ioreq.c >>>> @@ -99,6 +99,7 @@ static ioreq_t *get_ioreq(struct ioreq_server *s, struct >>>> vcpu *v) >>>> >>>> ASSERT((v == current) || !vcpu_runnable(v)); >>>> ASSERT(p != NULL); >>>> + BUILD_BUG_ON(HVM_MAX_VCPUS > (PAGE_SIZE / sizeof(struct ioreq))); >>> >>> ... does this even build on e.g. Arm? IOREQ_SERVER is a setting which can be >>> enabled (with EXPERT=y) also for non-x86. Yet HVM_MAX_VCPUS looks to be an >>> x86-only thing. (I then also wonder about some of what the description >>> says). >>> >>> Just to mention (no further change requested at this point, in this regard): >>> HVM_MAX_VCPUS being part of the public interface, we'll need to see whether >>> we >>> can sensibly retain that identifier to carry changed meaning once we up the >>> limit. The check here may therefore not trigger at that point; the hope then >>> is that while making respective changes, people would at least stumble >>> across >>> it by e.g. seeing it in grep output. >>> >> >> Apparently it doesn't build (debian-bookworm-gcc-arm32-randconfig >> catched it). >> ARM does provide MAX_VIRT_CPUS and GUEST_MAX_VCPUS which is 128 or >> lower, but that doesn't map (or not properly) with what we have in x86 >> (MAX_VIRT_CPUS=8192 is PV-specific, and GUEST_MAX_VCPUS doesn't exist). >> >> I am not sure what to do, looks like many things are redundant here. > > Maybe non-x86 could surface HVM_MAX_VCPUS as an alias of whatever they already > got, much like CONFIG_HVM exists also for Arm, and will likely need > introducing > for PPC and RISC-V (despite not being overly meaningful for non-x86)? > > Jan
I'd say this is the better choice, pending some non-x86 people acking the plan. Cheers, Alejandro
