Re: [Qemu-devel] [PATCH v15 1/3] i386: Fix up the Node id for CPUID_8000_001E

[Moger, Babu]

> -----Original Message-----
> From: address@hidden [mailto:address@hidden
> On Behalf Of Babu Moger
> Sent: Friday, June 15, 2018 12:16 PM
> To: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; Moger, Babu
> <address@hidden>
> Subject: [PATCH v15 1/3] i386: Fix up the Node id for CPUID_8000_001E
> 
> This is part of topoext support. To keep the compatibility, we need
> to support all the combination of nr_cores and nr_threads currently
> supported. With this combination, we might end up with more nodes than
> we can support with real hardware. We need to fix up the node id to
> accommodate more nodes here. We can achieve this by shifting the bits.
> 
> Signed-off-by: Babu Moger <address@hidden>
> ---
>  target/i386/cpu.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7a4484b..5246be4 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -19,6 +19,7 @@
> 
>  #include "qemu/osdep.h"
>  #include "qemu/cutils.h"
> +#include "qemu/bitops.h"
> 
>  #include "cpu.h"
>  #include "exec/exec-all.h"
> @@ -472,6 +473,8 @@ static void encode_topo_cpuid8000001e(CPUState
> *cs, X86CPU *cpu,
>                                         uint32_t *ecx, uint32_t *edx)
>  {
>      struct core_topology topo = {0};
> +    unsigned long nodes;
> +    int shift;
> 
>      build_core_topology(cs->nr_cores, cpu->core_id, &topo);
>      *eax = cpu->apic_id;
> @@ -504,7 +507,25 @@ static void encode_topo_cpuid8000001e(CPUState
> *cs, X86CPU *cpu,
>       *         2  Socket id
>       *       1:0  Node id
>       */
> -    *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
> topo.node_id;
> +    if (topo.num_nodes <= 4) {
> +        *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
> +                topo.node_id;
> +    } else {
> +        /*
> +         * Node id fix up. Actual hardware supports up to 4 nodes. But with
> +         * more than 32 cores, we may end up with more than 4 nodes.
> +         * Node id is a combination of socket id and node id. Only 
> requirement
> +         * here is that this number should be unique accross the system.
> +         * Shift the socket id to accommodate more nodes. We dont expect
> both
> +         * socket id and node id to be big number at the same time. This is 
> not
> +         * an ideal config but we need to to support it. Max bit we can have
> +         * here is 8.
> +         */
> +        nodes = topo.num_nodes - 1;
> +        shift = find_last_bit(&nodes, 8);
> +        *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << shift) |

Sorry. There is a bug here.  This should be (cpu->socket_id << (shift + 1) ). I 
will fix it.  Let me know the rest of the code.


> +                topo.node_id;
> +    }
>      *edx = 0;
>  }
> 
> --
> 1.8.3.1