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
