* [Qemu-devel] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface
@ 2019-02-28 20:04 Maxiwell S. Garcia
2019-03-05 19:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
2019-03-06 3:34 ` [Qemu-devel] " David Gibson
0 siblings, 2 replies; 4+ messages in thread
From: Maxiwell S. Garcia @ 2019-02-28 20:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Maxiwell S. Garcia, David Gibson, open list:sPAPR
This adds a handler for ibm,get-vpd RTAS calls, allowing pseries guest
to collect host information. It is disabled by default to avoid unwanted
information leakage. To enable it, use:
‘-M pseries,host-serial={passthrough|string},host-model={passthrough|string}’
Only the SE and TM keywords are returned at the moment.
SE for Machine or Cabinet Serial Number and
TM for Machine Type and Model
The SE and TM keywords are controlled by 'host-serial' and 'host-model'
options, respectively. In the case of 'passthrough', the SE shows the
host 'system-id' information and the TM shows the host 'model' information.
Powerpc-utils tools can dispatch RTAS calls to retrieve host
information using this ibm,get-vpd interface. The 'host-serial'
and 'host-model' nodes of device-tree hold the same information but
in a static manner, which is useless after a migration operation.
Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
---
hw/ppc/spapr_rtas.c | 121 +++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr.h | 17 +++++-
2 files changed, 137 insertions(+), 1 deletion(-)
Update v4:
* Allows enable/disable host-serial and host-model options individually
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 7a2cb786a3..785c453eb6 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -287,6 +287,123 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
rtas_st(rets, 0, ret);
}
+static inline int vpd_st(target_ulong addr, target_ulong len,
+ const void *val, uint16_t val_len)
+{
+ hwaddr phys = ppc64_phys_to_real(addr);
+ if (len < val_len) {
+ return RTAS_OUT_PARAM_ERROR;
+ }
+ cpu_physical_memory_write(phys, val, val_len);
+ return RTAS_OUT_SUCCESS;
+}
+
+static inline void vpd_ret(target_ulong rets, const int status,
+ const int next_seq_number, const int bytes_returned)
+{
+ rtas_st(rets, 0, status);
+ rtas_st(rets, 1, next_seq_number);
+ rtas_st(rets, 2, bytes_returned);
+}
+
+static void rtas_ibm_get_vpd_register_keywords(sPAPRMachineState *sm)
+{
+ sm->rtas_vpd_keywords = g_malloc0(sizeof(uint8_t) *
+ RTAS_IBM_VPD_KEYWORDS_MAX);
+
+ int i = 0;
+ if (sm->host_serial && !g_str_equal(sm->host_serial, "none")) {
+ sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_SE;
+ }
+ if (sm->host_model && !g_str_equal(sm->host_model, "none")) {
+ sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_TM;
+ }
+}
+
+static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
+ sPAPRMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ target_ulong loc_code_addr;
+ target_ulong work_area_addr;
+ target_ulong work_area_size;
+ target_ulong seq_number;
+ unsigned char loc_code = 0;
+ unsigned int next_seq_number = 1;
+ int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
+ int ret = RTAS_OUT_PARAM_ERROR;
+ char *buf = NULL;
+ char *vpd_field = NULL;
+ unsigned int vpd_field_len = 0;
+
+ /* RTAS not authorized if no keywords have been registered */
+ if (!spapr->rtas_vpd_keywords[0]) {
+ vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
+ return;
+ }
+
+ loc_code_addr = rtas_ld(args, 0);
+ work_area_addr = rtas_ld(args, 1);
+ work_area_size = rtas_ld(args, 2);
+ seq_number = rtas_ld(args, 3);
+
+ /* Specific Location Code is not supported and seq_number */
+ /* must be checked to avoid out of bound index error */
+ cpu_physical_memory_read(loc_code_addr, &loc_code, 1);
+ if ((loc_code != 0) || (seq_number <= 0) ||
+ (seq_number > RTAS_IBM_VPD_KEYWORDS_MAX)) {
+ vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
+ return;
+ }
+
+ switch (spapr->rtas_vpd_keywords[seq_number - 1]) {
+ case RTAS_IBM_VPD_KEYWORD_SE:
+ if (g_str_equal(spapr->host_serial, "passthrough")) {
+ /* -M host-serial=passthrough */
+ if (kvmppc_get_host_serial(&buf)) {
+ /* LoPAPR: SE for Machine or Cabinet Serial Number */
+ vpd_field = g_strdup_printf("SE %s", buf);
+ }
+ } else {
+ vpd_field = g_strdup_printf("SE %s", spapr->host_serial);
+ }
+ break;
+ case RTAS_IBM_VPD_KEYWORD_TM:
+ if (g_str_equal(spapr->host_model, "passthrough")) {
+ /* -M host-model=passthrough */
+ if (kvmppc_get_host_model(&buf)) {
+ /* LoPAPR: TM for Machine Type and Model */
+ vpd_field = g_strdup_printf("TM %s", buf);
+ }
+ } else {
+ vpd_field = g_strdup_printf("TM %s", spapr->host_model);
+ }
+ break;
+ }
+
+ if (vpd_field) {
+ vpd_field_len = strlen(vpd_field);
+ ret = vpd_st(work_area_addr, work_area_size,
+ vpd_field, vpd_field_len + 1);
+
+ if (ret == 0) {
+ next_seq_number = seq_number + 1;
+ if (spapr->rtas_vpd_keywords[next_seq_number - 1]) {
+ status = RTAS_IBM_GET_VPD_CONTINUE;
+ } else {
+ status = RTAS_IBM_GET_VPD_SUCCESS;
+ next_seq_number = 1;
+ }
+ }
+ }
+
+ vpd_ret(rets, status, next_seq_number, vpd_field_len);
+ g_free(vpd_field);
+ g_free(buf);
+}
+
static void rtas_ibm_os_term(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
uint32_t token, uint32_t nargs,
@@ -464,6 +581,8 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
fdt_strerror(ret));
exit(1);
}
+
+ rtas_ibm_get_vpd_register_keywords(spapr);
}
static void core_rtas_register_types(void)
@@ -485,6 +604,8 @@ static void core_rtas_register_types(void)
rtas_ibm_set_system_parameter);
spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
rtas_ibm_os_term);
+ spapr_rtas_register(RTAS_IBM_GET_VPD, "ibm,get-vpd",
+ rtas_ibm_get_vpd);
spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
rtas_set_power_level);
spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 59073a7579..ebf314a15c 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -153,6 +153,7 @@ struct sPAPRMachineState {
struct PPCTimebase tb;
bool has_graphics;
uint32_t vsmt; /* Virtual SMT mode (KVM's "core stride") */
+ uint8_t *rtas_vpd_keywords;
Notifier epow_notifier;
QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
@@ -608,14 +609,28 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
#define RTAS_IBM_CREATE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x27)
#define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
#define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
+#define RTAS_IBM_GET_VPD (RTAS_TOKEN_BASE + 0x2A)
-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2A)
+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
/* RTAS ibm,get-system-parameter token values */
#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
#define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE 42
#define RTAS_SYSPARM_UUID 48
+/* RTAS ibm,get-vpd status values */
+#define RTAS_IBM_GET_VPD_VPD_CHANGED -4
+#define RTAS_IBM_GET_VPD_PARAMETER_ERROR -3
+#define RTAS_IBM_GET_VPD_HARDWARE_ERROR -1
+#define RTAS_IBM_GET_VPD_SUCCESS 0
+#define RTAS_IBM_GET_VPD_CONTINUE 1
+
+/* RTAS ibm,get-vpd keywords index */
+#define RTAS_IBM_VPD_KEYWORD_SE 1
+#define RTAS_IBM_VPD_KEYWORD_TM 2
+
+#define RTAS_IBM_VPD_KEYWORDS_MAX 2
+
/* RTAS indicator/sensor types
*
* as defined by PAPR+ 2.7 7.3.5.4, Table 41
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface
2019-02-28 20:04 [Qemu-devel] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface Maxiwell S. Garcia
@ 2019-03-05 19:39 ` Murilo Opsfelder Araujo
2019-03-05 22:20 ` Maxiwell S. Garcia
2019-03-06 3:34 ` [Qemu-devel] " David Gibson
1 sibling, 1 reply; 4+ messages in thread
From: Murilo Opsfelder Araujo @ 2019-03-05 19:39 UTC (permalink / raw)
To: Maxiwell S. Garcia; +Cc: qemu-devel, open list:sPAPR, David Gibson
Hi, Maxiwell.
On Thu, Feb 28, 2019 at 05:04:37PM -0300, Maxiwell S. Garcia wrote:
> This adds a handler for ibm,get-vpd RTAS calls, allowing pseries guest
> to collect host information. It is disabled by default to avoid unwanted
> information leakage. To enable it, use:
>
> ‘-M pseries,host-serial={passthrough|string},host-model={passthrough|string}’
>
> Only the SE and TM keywords are returned at the moment.
> SE for Machine or Cabinet Serial Number and
> TM for Machine Type and Model
>
> The SE and TM keywords are controlled by 'host-serial' and 'host-model'
> options, respectively. In the case of 'passthrough', the SE shows the
> host 'system-id' information and the TM shows the host 'model' information.
>
> Powerpc-utils tools can dispatch RTAS calls to retrieve host
> information using this ibm,get-vpd interface. The 'host-serial'
> and 'host-model' nodes of device-tree hold the same information but
> in a static manner, which is useless after a migration operation.
>
> Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
> ---
> hw/ppc/spapr_rtas.c | 121 +++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 17 +++++-
> 2 files changed, 137 insertions(+), 1 deletion(-)
>
> Update v4:
> * Allows enable/disable host-serial and host-model options individually
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 7a2cb786a3..785c453eb6 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -287,6 +287,123 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
> rtas_st(rets, 0, ret);
> }
>
> +static inline int vpd_st(target_ulong addr, target_ulong len,
> + const void *val, uint16_t val_len)
> +{
> + hwaddr phys = ppc64_phys_to_real(addr);
> + if (len < val_len) {
> + return RTAS_OUT_PARAM_ERROR;
> + }
> + cpu_physical_memory_write(phys, val, val_len);
> + return RTAS_OUT_SUCCESS;
> +}
> +
> +static inline void vpd_ret(target_ulong rets, const int status,
> + const int next_seq_number, const int bytes_returned)
> +{
> + rtas_st(rets, 0, status);
> + rtas_st(rets, 1, next_seq_number);
> + rtas_st(rets, 2, bytes_returned);
> +}
> +
> +static void rtas_ibm_get_vpd_register_keywords(sPAPRMachineState *sm)
> +{
> + sm->rtas_vpd_keywords = g_malloc0(sizeof(uint8_t) *
> + RTAS_IBM_VPD_KEYWORDS_MAX);
> +
> + int i = 0;
> + if (sm->host_serial && !g_str_equal(sm->host_serial, "none")) {
> + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_SE;
> + }
> + if (sm->host_model && !g_str_equal(sm->host_model, "none")) {
> + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_TM;
> + }
> +}
> +
> +static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> + target_ulong loc_code_addr;
> + target_ulong work_area_addr;
> + target_ulong work_area_size;
> + target_ulong seq_number;
> + unsigned char loc_code = 0;
> + unsigned int next_seq_number = 1;
> + int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
> + int ret = RTAS_OUT_PARAM_ERROR;
> + char *buf = NULL;
> + char *vpd_field = NULL;
> + unsigned int vpd_field_len = 0;
> +
> + /* RTAS not authorized if no keywords have been registered */
> + if (!spapr->rtas_vpd_keywords[0]) {
> + vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
> + return;
> + }
> +
> + loc_code_addr = rtas_ld(args, 0);
> + work_area_addr = rtas_ld(args, 1);
> + work_area_size = rtas_ld(args, 2);
> + seq_number = rtas_ld(args, 3);
> +
> + /* Specific Location Code is not supported and seq_number */
> + /* must be checked to avoid out of bound index error */
> + cpu_physical_memory_read(loc_code_addr, &loc_code, 1);
> + if ((loc_code != 0) || (seq_number <= 0) ||
> + (seq_number > RTAS_IBM_VPD_KEYWORDS_MAX)) {
> + vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
> + return;
> + }
> +
> + switch (spapr->rtas_vpd_keywords[seq_number - 1]) {
Since seq_number comes from guest userspace, we shouldn't rely its
value will always be within the range of the array.
We could perhaps return RTAS_OUT_NOT_AUTHORIZED if `seq_number - 1`
extrapolates the array range that holds the registered keywords.
> + case RTAS_IBM_VPD_KEYWORD_SE:
> + if (g_str_equal(spapr->host_serial, "passthrough")) {
> + /* -M host-serial=passthrough */
> + if (kvmppc_get_host_serial(&buf)) {
> + /* LoPAPR: SE for Machine or Cabinet Serial Number */
> + vpd_field = g_strdup_printf("SE %s", buf);
> + }
> + } else {
> + vpd_field = g_strdup_printf("SE %s", spapr->host_serial);
> + }
> + break;
> + case RTAS_IBM_VPD_KEYWORD_TM:
> + if (g_str_equal(spapr->host_model, "passthrough")) {
> + /* -M host-model=passthrough */
> + if (kvmppc_get_host_model(&buf)) {
> + /* LoPAPR: TM for Machine Type and Model */
> + vpd_field = g_strdup_printf("TM %s", buf);
> + }
> + } else {
> + vpd_field = g_strdup_printf("TM %s", spapr->host_model);
> + }
> + break;
> + }
> +
> + if (vpd_field) {
> + vpd_field_len = strlen(vpd_field);
> + ret = vpd_st(work_area_addr, work_area_size,
> + vpd_field, vpd_field_len + 1);
> +
> + if (ret == 0) {
> + next_seq_number = seq_number + 1;
> + if (spapr->rtas_vpd_keywords[next_seq_number - 1]) {
> + status = RTAS_IBM_GET_VPD_CONTINUE;
> + } else {
> + status = RTAS_IBM_GET_VPD_SUCCESS;
> + next_seq_number = 1;
If spapr->rtas_vpd_keywords[next_seq_number - 1] doesn't exist,
shouldn't we set next_seq_number=RTAS_IBM_VPD_KEYWORDS_MAX here,
indicating next_seq_number reached the last registered keyword?
> + }
> + }
> + }
> +
> + vpd_ret(rets, status, next_seq_number, vpd_field_len);
> + g_free(vpd_field);
> + g_free(buf);
> +}
> +
> static void rtas_ibm_os_term(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -464,6 +581,8 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
> fdt_strerror(ret));
> exit(1);
> }
> +
> + rtas_ibm_get_vpd_register_keywords(spapr);
> }
>
> static void core_rtas_register_types(void)
> @@ -485,6 +604,8 @@ static void core_rtas_register_types(void)
> rtas_ibm_set_system_parameter);
> spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
> rtas_ibm_os_term);
> + spapr_rtas_register(RTAS_IBM_GET_VPD, "ibm,get-vpd",
> + rtas_ibm_get_vpd);
> spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
> rtas_set_power_level);
> spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 59073a7579..ebf314a15c 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -153,6 +153,7 @@ struct sPAPRMachineState {
> struct PPCTimebase tb;
> bool has_graphics;
> uint32_t vsmt; /* Virtual SMT mode (KVM's "core stride") */
> + uint8_t *rtas_vpd_keywords;
>
> Notifier epow_notifier;
> QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
> @@ -608,14 +609,28 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> #define RTAS_IBM_CREATE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x27)
> #define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
> #define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
> +#define RTAS_IBM_GET_VPD (RTAS_TOKEN_BASE + 0x2A)
>
> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2A)
> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
>
> /* RTAS ibm,get-system-parameter token values */
> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
> #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE 42
> #define RTAS_SYSPARM_UUID 48
>
> +/* RTAS ibm,get-vpd status values */
> +#define RTAS_IBM_GET_VPD_VPD_CHANGED -4
> +#define RTAS_IBM_GET_VPD_PARAMETER_ERROR -3
> +#define RTAS_IBM_GET_VPD_HARDWARE_ERROR -1
> +#define RTAS_IBM_GET_VPD_SUCCESS 0
> +#define RTAS_IBM_GET_VPD_CONTINUE 1
> +
> +/* RTAS ibm,get-vpd keywords index */
> +#define RTAS_IBM_VPD_KEYWORD_SE 1
> +#define RTAS_IBM_VPD_KEYWORD_TM 2
> +
> +#define RTAS_IBM_VPD_KEYWORDS_MAX 2
> +
> /* RTAS indicator/sensor types
> *
> * as defined by PAPR+ 2.7 7.3.5.4, Table 41
> --
> 2.20.1
>
>
--
Murilo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface
2019-03-05 19:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
@ 2019-03-05 22:20 ` Maxiwell S. Garcia
0 siblings, 0 replies; 4+ messages in thread
From: Maxiwell S. Garcia @ 2019-03-05 22:20 UTC (permalink / raw)
To: Murilo Opsfelder Araujo; +Cc: open list:sPAPR, qemu-devel, David Gibson
Hi Murilo,
On Tue, Mar 05, 2019 at 04:39:38PM -0300, Murilo Opsfelder Araujo wrote:
> Hi, Maxiwell.
>
> On Thu, Feb 28, 2019 at 05:04:37PM -0300, Maxiwell S. Garcia wrote:
> > This adds a handler for ibm,get-vpd RTAS calls, allowing pseries guest
> > to collect host information. It is disabled by default to avoid unwanted
> > information leakage. To enable it, use:
> >
> > ‘-M pseries,host-serial={passthrough|string},host-model={passthrough|string}’
> >
> > Only the SE and TM keywords are returned at the moment.
> > SE for Machine or Cabinet Serial Number and
> > TM for Machine Type and Model
> >
> > The SE and TM keywords are controlled by 'host-serial' and 'host-model'
> > options, respectively. In the case of 'passthrough', the SE shows the
> > host 'system-id' information and the TM shows the host 'model' information.
> >
> > Powerpc-utils tools can dispatch RTAS calls to retrieve host
> > information using this ibm,get-vpd interface. The 'host-serial'
> > and 'host-model' nodes of device-tree hold the same information but
> > in a static manner, which is useless after a migration operation.
> >
> > Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
> > ---
> > hw/ppc/spapr_rtas.c | 121 +++++++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/spapr.h | 17 +++++-
> > 2 files changed, 137 insertions(+), 1 deletion(-)
> >
> > Update v4:
> > * Allows enable/disable host-serial and host-model options individually
> >
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 7a2cb786a3..785c453eb6 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -287,6 +287,123 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
> > rtas_st(rets, 0, ret);
> > }
> >
> > +static inline int vpd_st(target_ulong addr, target_ulong len,
> > + const void *val, uint16_t val_len)
> > +{
> > + hwaddr phys = ppc64_phys_to_real(addr);
> > + if (len < val_len) {
> > + return RTAS_OUT_PARAM_ERROR;
> > + }
> > + cpu_physical_memory_write(phys, val, val_len);
> > + return RTAS_OUT_SUCCESS;
> > +}
> > +
> > +static inline void vpd_ret(target_ulong rets, const int status,
> > + const int next_seq_number, const int bytes_returned)
> > +{
> > + rtas_st(rets, 0, status);
> > + rtas_st(rets, 1, next_seq_number);
> > + rtas_st(rets, 2, bytes_returned);
> > +}
> > +
> > +static void rtas_ibm_get_vpd_register_keywords(sPAPRMachineState *sm)
> > +{
> > + sm->rtas_vpd_keywords = g_malloc0(sizeof(uint8_t) *
> > + RTAS_IBM_VPD_KEYWORDS_MAX);
> > +
> > + int i = 0;
> > + if (sm->host_serial && !g_str_equal(sm->host_serial, "none")) {
> > + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_SE;
> > + }
> > + if (sm->host_model && !g_str_equal(sm->host_model, "none")) {
> > + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_TM;
> > + }
> > +}
> > +
> > +static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
> > + sPAPRMachineState *spapr,
> > + uint32_t token, uint32_t nargs,
> > + target_ulong args,
> > + uint32_t nret, target_ulong rets)
> > +{
> > + target_ulong loc_code_addr;
> > + target_ulong work_area_addr;
> > + target_ulong work_area_size;
> > + target_ulong seq_number;
> > + unsigned char loc_code = 0;
> > + unsigned int next_seq_number = 1;
> > + int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
> > + int ret = RTAS_OUT_PARAM_ERROR;
> > + char *buf = NULL;
> > + char *vpd_field = NULL;
> > + unsigned int vpd_field_len = 0;
> > +
> > + /* RTAS not authorized if no keywords have been registered */
> > + if (!spapr->rtas_vpd_keywords[0]) {
> > + vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
> > + return;
> > + }
> > +
> > + loc_code_addr = rtas_ld(args, 0);
> > + work_area_addr = rtas_ld(args, 1);
> > + work_area_size = rtas_ld(args, 2);
> > + seq_number = rtas_ld(args, 3);
> > +
> > + /* Specific Location Code is not supported and seq_number */
> > + /* must be checked to avoid out of bound index error */
> > + cpu_physical_memory_read(loc_code_addr, &loc_code, 1);
> > + if ((loc_code != 0) || (seq_number <= 0) ||
> > + (seq_number > RTAS_IBM_VPD_KEYWORDS_MAX)) {
> > + vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
> > + return;
> > + }
> > +
> > + switch (spapr->rtas_vpd_keywords[seq_number - 1]) {
>
> Since seq_number comes from guest userspace, we shouldn't rely its
> value will always be within the range of the array.
>
> We could perhaps return RTAS_OUT_NOT_AUTHORIZED if `seq_number - 1`
> extrapolates the array range that holds the registered keywords.
>
The seq_number is tested in the 'if' statement before access
the rtas_vpd_keywords array and it's returns RTAS_IBM_GET_VPD_PARAMETER_ERROR
in case of out of bound index.
> > + case RTAS_IBM_VPD_KEYWORD_SE:
> > + if (g_str_equal(spapr->host_serial, "passthrough")) {
> > + /* -M host-serial=passthrough */
> > + if (kvmppc_get_host_serial(&buf)) {
> > + /* LoPAPR: SE for Machine or Cabinet Serial Number */
> > + vpd_field = g_strdup_printf("SE %s", buf);
> > + }
> > + } else {
> > + vpd_field = g_strdup_printf("SE %s", spapr->host_serial);
> > + }
> > + break;
> > + case RTAS_IBM_VPD_KEYWORD_TM:
> > + if (g_str_equal(spapr->host_model, "passthrough")) {
> > + /* -M host-model=passthrough */
> > + if (kvmppc_get_host_model(&buf)) {
> > + /* LoPAPR: TM for Machine Type and Model */
> > + vpd_field = g_strdup_printf("TM %s", buf);
> > + }
> > + } else {
> > + vpd_field = g_strdup_printf("TM %s", spapr->host_model);
> > + }
> > + break;
> > + }
> > +
> > + if (vpd_field) {
> > + vpd_field_len = strlen(vpd_field);
> > + ret = vpd_st(work_area_addr, work_area_size,
> > + vpd_field, vpd_field_len + 1);
> > +
> > + if (ret == 0) {
> > + next_seq_number = seq_number + 1;
> > + if (spapr->rtas_vpd_keywords[next_seq_number - 1]) {
> > + status = RTAS_IBM_GET_VPD_CONTINUE;
> > + } else {
> > + status = RTAS_IBM_GET_VPD_SUCCESS;
> > + next_seq_number = 1;
>
> If spapr->rtas_vpd_keywords[next_seq_number - 1] doesn't exist,
> shouldn't we set next_seq_number=RTAS_IBM_VPD_KEYWORDS_MAX here,
> indicating next_seq_number reached the last registered keyword?
>
The LoPAPR DRAFT v11 24March2016, page 239, says to return 1 if no more
calls are required.
> > + }
> > + }
> > + }
> > +
> > + vpd_ret(rets, status, next_seq_number, vpd_field_len);
> > + g_free(vpd_field);
> > + g_free(buf);
> > +}
> > +
> > static void rtas_ibm_os_term(PowerPCCPU *cpu,
> > sPAPRMachineState *spapr,
> > uint32_t token, uint32_t nargs,
> > @@ -464,6 +581,8 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
> > fdt_strerror(ret));
> > exit(1);
> > }
> > +
> > + rtas_ibm_get_vpd_register_keywords(spapr);
> > }
> >
> > static void core_rtas_register_types(void)
> > @@ -485,6 +604,8 @@ static void core_rtas_register_types(void)
> > rtas_ibm_set_system_parameter);
> > spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
> > rtas_ibm_os_term);
> > + spapr_rtas_register(RTAS_IBM_GET_VPD, "ibm,get-vpd",
> > + rtas_ibm_get_vpd);
> > spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
> > rtas_set_power_level);
> > spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 59073a7579..ebf314a15c 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -153,6 +153,7 @@ struct sPAPRMachineState {
> > struct PPCTimebase tb;
> > bool has_graphics;
> > uint32_t vsmt; /* Virtual SMT mode (KVM's "core stride") */
> > + uint8_t *rtas_vpd_keywords;
> >
> > Notifier epow_notifier;
> > QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
> > @@ -608,14 +609,28 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> > #define RTAS_IBM_CREATE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x27)
> > #define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
> > #define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
> > +#define RTAS_IBM_GET_VPD (RTAS_TOKEN_BASE + 0x2A)
> >
> > -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2A)
> > +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
> >
> > /* RTAS ibm,get-system-parameter token values */
> > #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
> > #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE 42
> > #define RTAS_SYSPARM_UUID 48
> >
> > +/* RTAS ibm,get-vpd status values */
> > +#define RTAS_IBM_GET_VPD_VPD_CHANGED -4
> > +#define RTAS_IBM_GET_VPD_PARAMETER_ERROR -3
> > +#define RTAS_IBM_GET_VPD_HARDWARE_ERROR -1
> > +#define RTAS_IBM_GET_VPD_SUCCESS 0
> > +#define RTAS_IBM_GET_VPD_CONTINUE 1
> > +
> > +/* RTAS ibm,get-vpd keywords index */
> > +#define RTAS_IBM_VPD_KEYWORD_SE 1
> > +#define RTAS_IBM_VPD_KEYWORD_TM 2
> > +
> > +#define RTAS_IBM_VPD_KEYWORDS_MAX 2
> > +
> > /* RTAS indicator/sensor types
> > *
> > * as defined by PAPR+ 2.7 7.3.5.4, Table 41
> > --
> > 2.20.1
> >
> >
>
> --
> Murilo
>
>
Thanks for review
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface
2019-02-28 20:04 [Qemu-devel] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface Maxiwell S. Garcia
2019-03-05 19:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
@ 2019-03-06 3:34 ` David Gibson
1 sibling, 0 replies; 4+ messages in thread
From: David Gibson @ 2019-03-06 3:34 UTC (permalink / raw)
To: Maxiwell S. Garcia; +Cc: qemu-devel, open list:sPAPR
[-- Attachment #1: Type: text/plain, Size: 10038 bytes --]
On Thu, Feb 28, 2019 at 05:04:37PM -0300, Maxiwell S. Garcia wrote:
> This adds a handler for ibm,get-vpd RTAS calls, allowing pseries guest
> to collect host information. It is disabled by default to avoid unwanted
> information leakage. To enable it, use:
>
> ‘-M pseries,host-serial={passthrough|string},host-model={passthrough|string}’
>
> Only the SE and TM keywords are returned at the moment.
> SE for Machine or Cabinet Serial Number and
> TM for Machine Type and Model
>
> The SE and TM keywords are controlled by 'host-serial' and 'host-model'
> options, respectively. In the case of 'passthrough', the SE shows the
> host 'system-id' information and the TM shows the host 'model' information.
>
> Powerpc-utils tools can dispatch RTAS calls to retrieve host
> information using this ibm,get-vpd interface. The 'host-serial'
> and 'host-model' nodes of device-tree hold the same information but
> in a static manner, which is useless after a migration operation.
>
> Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
> ---
> hw/ppc/spapr_rtas.c | 121 +++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 17 +++++-
> 2 files changed, 137 insertions(+), 1 deletion(-)
>
> Update v4:
> * Allows enable/disable host-serial and host-model options individually
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 7a2cb786a3..785c453eb6 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -287,6 +287,123 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
> rtas_st(rets, 0, ret);
> }
>
> +static inline int vpd_st(target_ulong addr, target_ulong len,
> + const void *val, uint16_t val_len)
> +{
> + hwaddr phys = ppc64_phys_to_real(addr);
> + if (len < val_len) {
> + return RTAS_OUT_PARAM_ERROR;
> + }
> + cpu_physical_memory_write(phys, val, val_len);
> + return RTAS_OUT_SUCCESS;
> +}
> +
> +static inline void vpd_ret(target_ulong rets, const int status,
> + const int next_seq_number, const int bytes_returned)
> +{
> + rtas_st(rets, 0, status);
> + rtas_st(rets, 1, next_seq_number);
> + rtas_st(rets, 2, bytes_returned);
> +}
> +
> +static void rtas_ibm_get_vpd_register_keywords(sPAPRMachineState *sm)
> +{
> + sm->rtas_vpd_keywords = g_malloc0(sizeof(uint8_t) *
> + RTAS_IBM_VPD_KEYWORDS_MAX);
> +
> + int i = 0;
> + if (sm->host_serial && !g_str_equal(sm->host_serial, "none")) {
> + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_SE;
> + }
> + if (sm->host_model && !g_str_equal(sm->host_model, "none")) {
> + sm->rtas_vpd_keywords[i++] = RTAS_IBM_VPD_KEYWORD_TM;
> + }
Creating this "index" of keywords, then constructing each keyword's
value as we step through it seems quite complex. It might be simpler
to just generate the whole VPD as an array of structs, indexed by
sequence number at reset time, then have get-vpd just copy that out to
the guest.
That said, the current approach does work, so I'm ok to leave it if
it's too much trouble to rework.
> +}
> +
> +static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> + target_ulong loc_code_addr;
> + target_ulong work_area_addr;
> + target_ulong work_area_size;
> + target_ulong seq_number;
> + unsigned char loc_code = 0;
> + unsigned int next_seq_number = 1;
> + int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
> + int ret = RTAS_OUT_PARAM_ERROR;
> + char *buf = NULL;
> + char *vpd_field = NULL;
> + unsigned int vpd_field_len = 0;
> +
> + /* RTAS not authorized if no keywords have been registered */
> + if (!spapr->rtas_vpd_keywords[0]) {
> + vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
> + return;
> + }
> +
> + loc_code_addr = rtas_ld(args, 0);
> + work_area_addr = rtas_ld(args, 1);
> + work_area_size = rtas_ld(args, 2);
> + seq_number = rtas_ld(args, 3);
> +
> + /* Specific Location Code is not supported and seq_number */
> + /* must be checked to avoid out of bound index error */
> + cpu_physical_memory_read(loc_code_addr, &loc_code, 1);
> + if ((loc_code != 0) || (seq_number <= 0) ||
> + (seq_number > RTAS_IBM_VPD_KEYWORDS_MAX)) {
> + vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
> + return;
> + }
> +
> + switch (spapr->rtas_vpd_keywords[seq_number - 1]) {
> + case RTAS_IBM_VPD_KEYWORD_SE:
> + if (g_str_equal(spapr->host_serial, "passthrough")) {
> + /* -M host-serial=passthrough */
> + if (kvmppc_get_host_serial(&buf)) {
> + /* LoPAPR: SE for Machine or Cabinet Serial Number */
> + vpd_field = g_strdup_printf("SE %s", buf);
> + }
> + } else {
> + vpd_field = g_strdup_printf("SE %s", spapr->host_serial);
> + }
I'd like to see a helper function to avoid duplicating this logic
between the vpd code and the device tree generation.
> + break;
> + case RTAS_IBM_VPD_KEYWORD_TM:
> + if (g_str_equal(spapr->host_model, "passthrough")) {
> + /* -M host-model=passthrough */
> + if (kvmppc_get_host_model(&buf)) {
> + /* LoPAPR: TM for Machine Type and Model */
> + vpd_field = g_strdup_printf("TM %s", buf);
> + }
> + } else {
> + vpd_field = g_strdup_printf("TM %s", spapr->host_model);
> + }
Same here.
> + break;
> + }
> +
> + if (vpd_field) {
> + vpd_field_len = strlen(vpd_field);
> + ret = vpd_st(work_area_addr, work_area_size,
> + vpd_field, vpd_field_len + 1);
> +
> + if (ret == 0) {
> + next_seq_number = seq_number + 1;
> + if (spapr->rtas_vpd_keywords[next_seq_number - 1]) {
This isn't safe. You only have exactly as many entries in your table
as the maximum number of populated entries (2). So, if the table is
full, this will read base the end of the table.
> + status = RTAS_IBM_GET_VPD_CONTINUE;
> + } else {
> + status = RTAS_IBM_GET_VPD_SUCCESS;
> + next_seq_number = 1;
> + }
> + }
> + }
> +
> + vpd_ret(rets, status, next_seq_number, vpd_field_len);
> + g_free(vpd_field);
> + g_free(buf);
> +}
> +
> static void rtas_ibm_os_term(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -464,6 +581,8 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
> fdt_strerror(ret));
> exit(1);
> }
> +
> + rtas_ibm_get_vpd_register_keywords(spapr);
> }
>
> static void core_rtas_register_types(void)
> @@ -485,6 +604,8 @@ static void core_rtas_register_types(void)
> rtas_ibm_set_system_parameter);
> spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
> rtas_ibm_os_term);
> + spapr_rtas_register(RTAS_IBM_GET_VPD, "ibm,get-vpd",
> + rtas_ibm_get_vpd);
> spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
> rtas_set_power_level);
> spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 59073a7579..ebf314a15c 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -153,6 +153,7 @@ struct sPAPRMachineState {
> struct PPCTimebase tb;
> bool has_graphics;
> uint32_t vsmt; /* Virtual SMT mode (KVM's "core stride") */
> + uint8_t *rtas_vpd_keywords;
>
> Notifier epow_notifier;
> QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
> @@ -608,14 +609,28 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> #define RTAS_IBM_CREATE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x27)
> #define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
> #define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
> +#define RTAS_IBM_GET_VPD (RTAS_TOKEN_BASE + 0x2A)
>
> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2A)
> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
>
> /* RTAS ibm,get-system-parameter token values */
> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
> #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE 42
> #define RTAS_SYSPARM_UUID 48
>
> +/* RTAS ibm,get-vpd status values */
> +#define RTAS_IBM_GET_VPD_VPD_CHANGED -4
> +#define RTAS_IBM_GET_VPD_PARAMETER_ERROR -3
> +#define RTAS_IBM_GET_VPD_HARDWARE_ERROR -1
> +#define RTAS_IBM_GET_VPD_SUCCESS 0
> +#define RTAS_IBM_GET_VPD_CONTINUE 1
> +
> +/* RTAS ibm,get-vpd keywords index */
> +#define RTAS_IBM_VPD_KEYWORD_SE 1
> +#define RTAS_IBM_VPD_KEYWORD_TM 2
> +
> +#define RTAS_IBM_VPD_KEYWORDS_MAX 2
AFAICT, these values aren't designated by PAPR, they're just internal.
In which case they seem like an unnecessary extra layer of
indirection. And the fact that they'll be mostly-but-not-always equal
to the sequence_number sounds like a recipe for confusion.
> +
> /* RTAS indicator/sensor types
> *
> * as defined by PAPR+ 2.7 7.3.5.4, Table 41
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-06 3:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-28 20:04 [Qemu-devel] [PATCH v4] spapr-rtas: add ibm, get-vpd RTAS interface Maxiwell S. Garcia
2019-03-05 19:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
2019-03-05 22:20 ` Maxiwell S. Garcia
2019-03-06 3:34 ` [Qemu-devel] " David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.