[PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs

Salil Mehta via posted 29 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs
Posted by Salil Mehta via 5 months, 2 weeks ago
This patch adds various utility functions that may be required to fetch or check
the state of possible vCPUs. It also introduces the concept of *disabled* vCPUs,
which are part of the *possible* vCPUs but are not enabled. This state will be
used during machine initialization and later during the plugging or unplugging
of vCPUs. We release the QOM CPU objects for all disabled vCPUs.

Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 cpu-common.c          | 31 +++++++++++++++++++++++++
 include/hw/core/cpu.h | 54 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/cpu-common.c b/cpu-common.c
index ce78273af5..49d2a50835 100644
--- a/cpu-common.c
+++ b/cpu-common.c
@@ -24,6 +24,7 @@
 #include "sysemu/cpus.h"
 #include "qemu/lockable.h"
 #include "trace/trace-root.h"
+#include "hw/boards.h"
 
 QemuMutex qemu_cpu_list_lock;
 static QemuCond exclusive_cond;
@@ -107,6 +108,36 @@ void cpu_list_remove(CPUState *cpu)
     cpu_list_generation_id++;
 }
 
+CPUState *qemu_get_possible_cpu(int index)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    const CPUArchIdList *possible_cpus = ms->possible_cpus;
+
+    assert((index >= 0) && (index < possible_cpus->len));
+
+    return CPU(possible_cpus->cpus[index].cpu);
+}
+
+bool qemu_present_cpu(CPUState *cpu)
+{
+    return cpu;
+}
+
+bool qemu_enabled_cpu(CPUState *cpu)
+{
+    return cpu && !cpu->disabled;
+}
+
+uint64_t qemu_get_cpu_archid(int cpu_index)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    const CPUArchIdList *possible_cpus = ms->possible_cpus;
+
+    assert((cpu_index >= 0) && (cpu_index < possible_cpus->len));
+
+    return possible_cpus->cpus[cpu_index].arch_id;
+}
+
 CPUState *qemu_get_cpu(int index)
 {
     CPUState *cpu;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 60b160d0b4..60b4778da9 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -528,6 +528,18 @@ struct CPUState {
     CPUPluginState *plugin_state;
 #endif
 
+    /*
+     * Some architectures do not allow the *presence* of vCPUs to be changed
+     * after the guest has booted, based on information specified by the
+     * VMM/firmware via ACPI MADT at boot time. Thus, to enable vCPU hotplug on
+     * these architectures, possible vCPUs can have a CPUState object in a
+     * 'disabled' state or may not have a CPUState object at all. This is
+     * possible when vCPU hotplug is supported, and vCPUs are
+     * 'yet-to-be-plugged' in the QOM or have been hot-unplugged. By default,
+     * every CPUState is enabled across all architectures.
+     */
+    bool disabled;
+
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index;
     int cluster_index;
@@ -914,6 +926,48 @@ static inline bool cpu_in_exclusive_context(const CPUState *cpu)
  */
 CPUState *qemu_get_cpu(int index);
 
+/**
+ * qemu_get_possible_cpu:
+ * @index: The CPUState@cpu_index value of the CPU to obtain.
+ *         Input index MUST be in range [0, Max Possible CPUs)
+ *
+ * If CPUState object exists,then it gets a CPU matching
+ * @index in the possible CPU array.
+ *
+ * Returns: The possible CPU or %NULL if CPU does not exist.
+ */
+CPUState *qemu_get_possible_cpu(int index);
+
+/**
+ * qemu_present_cpu:
+ * @cpu: The vCPU to check
+ *
+ * Checks if the vCPU is amongst the present possible vcpus.
+ *
+ * Returns: True if it is present possible vCPU else false
+ */
+bool qemu_present_cpu(CPUState *cpu);
+
+/**
+ * qemu_enabled_cpu:
+ * @cpu: The vCPU to check
+ *
+ * Checks if the vCPU is enabled.
+ *
+ * Returns: True if it is 'enabled' else false
+ */
+bool qemu_enabled_cpu(CPUState *cpu);
+
+/**
+ * qemu_get_cpu_archid:
+ * @cpu_index: possible vCPU for which arch-id needs to be retreived
+ *
+ * Fetches the vCPU arch-id from the present possible vCPUs.
+ *
+ * Returns: arch-id of the possible vCPU
+ */
+uint64_t qemu_get_cpu_archid(int cpu_index);
+
 /**
  * cpu_exists:
  * @id: Guest-exposed CPU ID to lookup.
-- 
2.34.1
Re: [PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs
Posted by Gavin Shan 3 months, 2 weeks ago
On 6/14/24 9:36 AM, Salil Mehta wrote:
> This patch adds various utility functions that may be required to fetch or check
> the state of possible vCPUs. It also introduces the concept of *disabled* vCPUs,
> which are part of the *possible* vCPUs but are not enabled. This state will be
> used during machine initialization and later during the plugging or unplugging
> of vCPUs. We release the QOM CPU objects for all disabled vCPUs.
> 
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>   cpu-common.c          | 31 +++++++++++++++++++++++++
>   include/hw/core/cpu.h | 54 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 85 insertions(+)
> 
> diff --git a/cpu-common.c b/cpu-common.c
> index ce78273af5..49d2a50835 100644
> --- a/cpu-common.c
> +++ b/cpu-common.c
> @@ -24,6 +24,7 @@
>   #include "sysemu/cpus.h"
>   #include "qemu/lockable.h"
>   #include "trace/trace-root.h"
> +#include "hw/boards.h"
>   
>   QemuMutex qemu_cpu_list_lock;
>   static QemuCond exclusive_cond;
> @@ -107,6 +108,36 @@ void cpu_list_remove(CPUState *cpu)
>       cpu_list_generation_id++;
>   }
>   
> +CPUState *qemu_get_possible_cpu(int index)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const CPUArchIdList *possible_cpus = ms->possible_cpus;
> +
> +    assert((index >= 0) && (index < possible_cpus->len));
> +
> +    return CPU(possible_cpus->cpus[index].cpu);
> +}
> +
> +bool qemu_present_cpu(CPUState *cpu)
> +{
> +    return cpu;
> +}
> +
> +bool qemu_enabled_cpu(CPUState *cpu)
> +{
> +    return cpu && !cpu->disabled;
> +}
> +
> +uint64_t qemu_get_cpu_archid(int cpu_index)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const CPUArchIdList *possible_cpus = ms->possible_cpus;
> +
> +    assert((cpu_index >= 0) && (cpu_index < possible_cpus->len));
> +
> +    return possible_cpus->cpus[cpu_index].arch_id;
> +}
> +
>   CPUState *qemu_get_cpu(int index)
>   {
>       CPUState *cpu;
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 60b160d0b4..60b4778da9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -528,6 +528,18 @@ struct CPUState {
>       CPUPluginState *plugin_state;
>   #endif
>   
> +    /*
> +     * Some architectures do not allow the *presence* of vCPUs to be changed
> +     * after the guest has booted, based on information specified by the
> +     * VMM/firmware via ACPI MADT at boot time. Thus, to enable vCPU hotplug on
> +     * these architectures, possible vCPUs can have a CPUState object in a
> +     * 'disabled' state or may not have a CPUState object at all. This is
> +     * possible when vCPU hotplug is supported, and vCPUs are
> +     * 'yet-to-be-plugged' in the QOM or have been hot-unplugged. By default,
> +     * every CPUState is enabled across all architectures.
> +     */
> +    bool disabled;
> +

The information to determine vCPU's state has been distributed to two data structs:
MachineState::possible_cpus::cpus[] and CPUState. Why not just to maintain the vCPU's
state from MachineState::possible_cpus::cpus[]? For example, adding a new field, or
something similiar, to 'struct CPUArchId' as below.

typedef struct CPUArchId {
#define CPU_ARCH_ID_FLAG_PRESENT	(1 << 0)
#define CPU_ARCH_ID_FLAG_ENABLED        (1 << 1)
     uint32_t flags;
        :
} CPUArchId;

In order to determine if a CPUState instance is enabled or not. CPUState::cpu_index
is used as the index to MachineState::possible_cpus::cpus[]. The flags can be parsed
to determine the vCPU's state.

>       /* TODO Move common fields from CPUArchState here. */
>       int cpu_index;
>       int cluster_index;
> @@ -914,6 +926,48 @@ static inline bool cpu_in_exclusive_context(const CPUState *cpu)
>    */
>   CPUState *qemu_get_cpu(int index);
>   
> +/**
> + * qemu_get_possible_cpu:
> + * @index: The CPUState@cpu_index value of the CPU to obtain.
> + *         Input index MUST be in range [0, Max Possible CPUs)
> + *
> + * If CPUState object exists,then it gets a CPU matching
> + * @index in the possible CPU array.
> + *
> + * Returns: The possible CPU or %NULL if CPU does not exist.
> + */
> +CPUState *qemu_get_possible_cpu(int index);
> +
> +/**
> + * qemu_present_cpu:
> + * @cpu: The vCPU to check
> + *
> + * Checks if the vCPU is amongst the present possible vcpus.
> + *
> + * Returns: True if it is present possible vCPU else false
> + */
> +bool qemu_present_cpu(CPUState *cpu);
> +
> +/**
> + * qemu_enabled_cpu:
> + * @cpu: The vCPU to check
> + *
> + * Checks if the vCPU is enabled.
> + *
> + * Returns: True if it is 'enabled' else false
> + */
> +bool qemu_enabled_cpu(CPUState *cpu);
> +
> +/**
> + * qemu_get_cpu_archid:
> + * @cpu_index: possible vCPU for which arch-id needs to be retreived
> + *
> + * Fetches the vCPU arch-id from the present possible vCPUs.
> + *
> + * Returns: arch-id of the possible vCPU
> + */
> +uint64_t qemu_get_cpu_archid(int cpu_index);
> +
>   /**
>    * cpu_exists:
>    * @id: Guest-exposed CPU ID to lookup.

Thanks,
Gavin
回复: [PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs
Posted by liu ping 3 months, 2 weeks ago
unsubscribe
________________________________
发件人: qemu-devel-bounces+liuping24=outlook.com@nongnu.org <qemu-devel-bounces+liuping24=outlook.com@nongnu.org> 代表 Gavin Shan <gshan@redhat.com>
发送时间: 2024年8月11日 21:59
收件人: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>; qemu-arm@nongnu.org <qemu-arm@nongnu.org>; mst@redhat.com <mst@redhat.com>
抄送: maz@kernel.org <maz@kernel.org>; jean-philippe@linaro.org <jean-philippe@linaro.org>; jonathan.cameron@huawei.com <jonathan.cameron@huawei.com>; lpieralisi@kernel.org <lpieralisi@kernel.org>; peter.maydell@linaro.org <peter.maydell@linaro.org>; richard.henderson@linaro.org <richard.henderson@linaro.org>; imammedo@redhat.com <imammedo@redhat.com>; andrew.jones@linux.dev <andrew.jones@linux.dev>; david@redhat.com <david@redhat.com>; philmd@linaro.org <philmd@linaro.org>; eric.auger@redhat.com <eric.auger@redhat.com>; will@kernel.org <will@kernel.org>; ardb@kernel.org <ardb@kernel.org>; oliver.upton@linux.dev <oliver.upton@linux.dev>; pbonzini@redhat.com <pbonzini@redhat.com>; rafael@kernel.org <rafael@kernel.org>; borntraeger@linux.ibm.com <borntraeger@linux.ibm.com>; alex.bennee@linaro.org <alex.bennee@linaro.org>; npiggin@gmail.com <npiggin@gmail.com>; harshpb@linux.ibm.com <harshpb@linux.ibm.com>; linux@armlinux.org.uk <linux@armlinux.org.uk>; darren@os.amperecomputing.com <darren@os.amperecomputing.com>; ilkka@os.amperecomputing.com <ilkka@os.amperecomputing.com>; vishnu@os.amperecomputing.com <vishnu@os.amperecomputing.com>; karl.heubaum@oracle.com <karl.heubaum@oracle.com>; miguel.luis@oracle.com <miguel.luis@oracle.com>; salil.mehta@opnsrc.net <salil.mehta@opnsrc.net>; zhukeqian1@huawei.com <zhukeqian1@huawei.com>; wangxiongfeng2@huawei.com <wangxiongfeng2@huawei.com>; wangyanan55@huawei.com <wangyanan55@huawei.com>; jiakernel2@gmail.com <jiakernel2@gmail.com>; maobibo@loongson.cn <maobibo@loongson.cn>; lixianglai@loongson.cn <lixianglai@loongson.cn>; shahuang@redhat.com <shahuang@redhat.com>; zhao1.liu@intel.com <zhao1.liu@intel.com>; linuxarm@huawei.com <linuxarm@huawei.com>
主题: Re: [PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs

On 6/14/24 9:36 AM, Salil Mehta wrote:
> This patch adds various utility functions that may be required to fetch or check
> the state of possible vCPUs. It also introduces the concept of *disabled* vCPUs,
> which are part of the *possible* vCPUs but are not enabled. This state will be
> used during machine initialization and later during the plugging or unplugging
> of vCPUs. We release the QOM CPU objects for all disabled vCPUs.
>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>   cpu-common.c          | 31 +++++++++++++++++++++++++
>   include/hw/core/cpu.h | 54 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 85 insertions(+)
>
> diff --git a/cpu-common.c b/cpu-common.c
> index ce78273af5..49d2a50835 100644
> --- a/cpu-common.c
> +++ b/cpu-common.c
> @@ -24,6 +24,7 @@
>   #include "sysemu/cpus.h"
>   #include "qemu/lockable.h"
>   #include "trace/trace-root.h"
> +#include "hw/boards.h"
>
>   QemuMutex qemu_cpu_list_lock;
>   static QemuCond exclusive_cond;
> @@ -107,6 +108,36 @@ void cpu_list_remove(CPUState *cpu)
>       cpu_list_generation_id++;
>   }
>
> +CPUState *qemu_get_possible_cpu(int index)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const CPUArchIdList *possible_cpus = ms->possible_cpus;
> +
> +    assert((index >= 0) && (index < possible_cpus->len));
> +
> +    return CPU(possible_cpus->cpus[index].cpu);
> +}
> +
> +bool qemu_present_cpu(CPUState *cpu)
> +{
> +    return cpu;
> +}
> +
> +bool qemu_enabled_cpu(CPUState *cpu)
> +{
> +    return cpu && !cpu->disabled;
> +}
> +
> +uint64_t qemu_get_cpu_archid(int cpu_index)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const CPUArchIdList *possible_cpus = ms->possible_cpus;
> +
> +    assert((cpu_index >= 0) && (cpu_index < possible_cpus->len));
> +
> +    return possible_cpus->cpus[cpu_index].arch_id;
> +}
> +
>   CPUState *qemu_get_cpu(int index)
>   {
>       CPUState *cpu;
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 60b160d0b4..60b4778da9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -528,6 +528,18 @@ struct CPUState {
>       CPUPluginState *plugin_state;
>   #endif
>
> +    /*
> +     * Some architectures do not allow the *presence* of vCPUs to be changed
> +     * after the guest has booted, based on information specified by the
> +     * VMM/firmware via ACPI MADT at boot time. Thus, to enable vCPU hotplug on
> +     * these architectures, possible vCPUs can have a CPUState object in a
> +     * 'disabled' state or may not have a CPUState object at all. This is
> +     * possible when vCPU hotplug is supported, and vCPUs are
> +     * 'yet-to-be-plugged' in the QOM or have been hot-unplugged. By default,
> +     * every CPUState is enabled across all architectures.
> +     */
> +    bool disabled;
> +

The information to determine vCPU's state has been distributed to two data structs:
MachineState::possible_cpus::cpus[] and CPUState. Why not just to maintain the vCPU's
state from MachineState::possible_cpus::cpus[]? For example, adding a new field, or
something similiar, to 'struct CPUArchId' as below.

typedef struct CPUArchId {
#define CPU_ARCH_ID_FLAG_PRESENT        (1 << 0)
#define CPU_ARCH_ID_FLAG_ENABLED        (1 << 1)
     uint32_t flags;
        :
} CPUArchId;

In order to determine if a CPUState instance is enabled or not. CPUState::cpu_index
is used as the index to MachineState::possible_cpus::cpus[]. The flags can be parsed
to determine the vCPU's state.

>       /* TODO Move common fields from CPUArchState here. */
>       int cpu_index;
>       int cluster_index;
> @@ -914,6 +926,48 @@ static inline bool cpu_in_exclusive_context(const CPUState *cpu)
>    */
>   CPUState *qemu_get_cpu(int index);
>
> +/**
> + * qemu_get_possible_cpu:
> + * @index: The CPUState@cpu_index value of the CPU to obtain.
> + *         Input index MUST be in range [0, Max Possible CPUs)
> + *
> + * If CPUState object exists,then it gets a CPU matching
> + * @index in the possible CPU array.
> + *
> + * Returns: The possible CPU or %NULL if CPU does not exist.
> + */
> +CPUState *qemu_get_possible_cpu(int index);
> +
> +/**
> + * qemu_present_cpu:
> + * @cpu: The vCPU to check
> + *
> + * Checks if the vCPU is amongst the present possible vcpus.
> + *
> + * Returns: True if it is present possible vCPU else false
> + */
> +bool qemu_present_cpu(CPUState *cpu);
> +
> +/**
> + * qemu_enabled_cpu:
> + * @cpu: The vCPU to check
> + *
> + * Checks if the vCPU is enabled.
> + *
> + * Returns: True if it is 'enabled' else false
> + */
> +bool qemu_enabled_cpu(CPUState *cpu);
> +
> +/**
> + * qemu_get_cpu_archid:
> + * @cpu_index: possible vCPU for which arch-id needs to be retreived
> + *
> + * Fetches the vCPU arch-id from the present possible vCPUs.
> + *
> + * Returns: arch-id of the possible vCPU
> + */
> +uint64_t qemu_get_cpu_archid(int cpu_index);
> +
>   /**
>    * cpu_exists:
>    * @id: Guest-exposed CPU ID to lookup.

Thanks,
Gavin


Re: [PATCH RFC V3 02/29] cpu-common: Add common CPU utility for possible vCPUs
Posted by Nicholas Piggin 4 months, 3 weeks ago
On Fri Jun 14, 2024 at 9:36 AM AEST, Salil Mehta wrote:

[...]

> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 60b160d0b4..60b4778da9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h

[...]

> +/**
> + * qemu_get_cpu_archid:
> + * @cpu_index: possible vCPU for which arch-id needs to be retreived
> + *
> + * Fetches the vCPU arch-id from the present possible vCPUs.
> + *
> + * Returns: arch-id of the possible vCPU
> + */
> +uint64_t qemu_get_cpu_archid(int cpu_index);

Not sure if blind... I can't see where this is used.

I'd be interested to see why it needs to be in non-arch code,
presumably it's only relevant to arch specific code. I'm
guessing ACPI needs it, but then could it be put into some
ACPI state or helper?

Thanks,
Nick