qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 1/9] hw: encode accessing CPU index in MemTxAttrs


From: Richard Henderson
Subject: Re: [PATCH v1 1/9] hw: encode accessing CPU index in MemTxAttrs
Date: Mon, 26 Sep 2022 07:33:13 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

On 9/25/22 13:02, Alex Bennée wrote:
I'm not keen on adding another field like this.

Hmm I thought it was unavoidable from Edgar's comment:

   "CPU's can also have a Master-IDs (Requester IDs) which are unrelated to
   the Clusters CPU index. This is used for example in the Xilinx ZynqMP
   and Xilinx Versal and the XMPU (Memory Protection Units).

   Anyway, I think this approach is an improvement from the current state
   but would rather see a new separate field from requester_id. Overloading
   requester_id will break some of our use-cases (in the Xilinx tree)..."

Of course we don't have to care about external use cases but it seemed
to indicate we might need both.

I missed that one.

What bounds our max number of cpus at the moment?  We use "int" in
struct CPUCore, but that's almost certainly for convenience.

target/s390x/cpu.h:#define S390_MAX_CPUS 248
hw/i386/pc_piix.c:    m->max_cpus = HVM_MAX_VCPUS;

hw/i386/pc_q35.c:    m->max_cpus = 288;

hw/arm/virt.c:    mc->max_cpus = 512;

hw/arm/sbsa-ref.c:    mc->max_cpus = 512;

hw/i386/microvm.c:    mc->max_cpus = 288;

hw/ppc/spapr.c:    mc->max_cpus = INT32_MAX;


Most of these are nicely bounded, but HVM_MAX_VCPUS is a magic number
from Xen, and ppc appears to be prepared for 31 bits worth of cpus.

 From 5642e4513e (spapr.c: do not use MachineClass::max_cpus to limit
CPUs) I think it is being a little optimistic. Even with the beefiest
hosts you start to see diminishing returns by ~12 vCPUs and it won't
take long before each extra vCPU just slows you down.

Ok, I guess. If we decided to add an assert that the cpuid fit in this field, we'd want to revisit this, I think. Not an issue for the moment.

I was confused by the last comment because I forgot the TLBs are not
shared between cores. So I can just bang:

     MemTxAttrs attrs = { .cpu_index = cs->cpu_index };

into arm_cpu_tlb_fill and be done with it?

Yes, it looks like it. I don't see any bulk overwrite of attrs in get_phys_addr and subroutines. Mostly modifications of attrs->secure, as expected.


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]