[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND 10/15] ppc: spapr: Initialize the GSB Elements lookup
From: |
Nicholas Piggin |
Subject: |
Re: [PATCH RESEND 10/15] ppc: spapr: Initialize the GSB Elements lookup table. |
Date: |
Thu, 07 Sep 2023 13:01:41 +1000 |
Might be good to add a common nested: prefix to all patches actually.
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> This is a first step towards enabling support for nested PAPR hcalls for
> providing the get/set of various Guest State Buffer (GSB) elements via
> h_guest_[g|s]et_state hcalls. This enables for identifying correct
> callbacks for get/set for each of the elements supported via
> h_guest_[g|s]et_state hcalls, support for which is added in next patch.
Changelog could use work.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> hw/ppc/spapr_hcall.c | 1 +
> hw/ppc/spapr_nested.c | 487 ++++++++++++++++++++++++++++++++++
> include/hw/ppc/ppc.h | 2 +
> include/hw/ppc/spapr_nested.h | 102 +++++++
> 4 files changed, 592 insertions(+)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 9b1f225d4a..ca609cb5a4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1580,6 +1580,7 @@ static void hypercall_register_types(void)
> spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
>
> spapr_register_nested();
> + init_nested();
This is for hcall registration, not general subsystem init I think.
Arguably not sure if it matters, it just looks odd for everything
else to be an hcall except this. I would just add a new init
function.
And actually now I look closer at this, I would not do your papr
hcall init in the cap apply function, if it is possible to do
inside spapr_register_nested(), then that function could look at
which caps are enabled and register the appropriate hcalls. Then
no change to move this into cap code.
> }
>
> type_init(hypercall_register_types)
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index e7956685af..6fbb1bcb02 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
[snip]
My eyes are going square, I'll review this later.
> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> index e095c002dc..d7acc28d17 100644
> --- a/include/hw/ppc/ppc.h
> +++ b/include/hw/ppc/ppc.h
> @@ -33,6 +33,8 @@ struct ppc_tb_t {
> QEMUTimer *decr_timer;
> /* Hypervisor decrementer management */
> uint64_t hdecr_next; /* Tick for next hdecr interrupt */
> + /* TB that HDEC should fire and return ctrl back to the Host partition */
> + uint64_t hdecr_expiry_tb;
Why is this here?
> QEMUTimer *hdecr_timer;
> int64_t purr_offset;
> void *opaque;
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index 2e8c6ba1ca..3c0d6a486e 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
[snip]
>
> +struct guest_state_element_type {
> + uint16_t id;
> + int size;
> +#define GUEST_STATE_ELEMENT_TYPE_FLAG_GUEST_WIDE 0x1
> +#define GUEST_STATE_ELEMENT_TYPE_FLAG_READ_ONLY 0x2
> + uint16_t flags;
> + void *(*location)(SpaprMachineStateNestedGuest *, target_ulong);
> + size_t offset;
> + void (*copy)(void *, void *, bool);
> + uint64_t mask;
> +};
I have to wonder whether this is the best way to go. Having
these indicrect function calls and array of "ops" like this
might be limiting the compiler. I wonder if it should just
be done in a switch table, which is how most interpreters
I've seen (which admittedly is not many) seem to do it.
Thanks,
Nick
- [PATCH RESEND 09/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_CREATE_VCPU, (continued)
- [PATCH RESEND 09/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_CREATE_VCPU, Harsh Prateek Bora, 2023/09/06
- [PATCH RESEND 15/15] ppc: spapr: Document Nested PAPR API, Harsh Prateek Bora, 2023/09/06
- [PATCH RESEND 04/15] ppc: spapr: Start using nested.api for nested kvm-hv api, Harsh Prateek Bora, 2023/09/06
- [PATCH RESEND 11/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_[GET|SET]_STATE, Harsh Prateek Bora, 2023/09/06
- [PATCH RESEND 10/15] ppc: spapr: Initialize the GSB Elements lookup table., Harsh Prateek Bora, 2023/09/06
- Re: [PATCH RESEND 10/15] ppc: spapr: Initialize the GSB Elements lookup table.,
Nicholas Piggin <=