From: Alexander Graf <agraf@suse.de>
To: Gavin Shan <gwshan@linux.vnet.ibm.com>, qemu-devel@nongnu.org
Cc: aik@ozlabs.ru, alex.williamson@redhat.com, qiudayu@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH v10 1/3] sPAPR: Implement EEH RTAS calls
Date: Tue, 24 Jun 2014 16:43:23 +0200 [thread overview]
Message-ID: <53A98E8B.7010302@suse.de> (raw)
In-Reply-To: <1402365785-31620-2-git-send-email-gwshan@linux.vnet.ibm.com>
On 10.06.14 04:03, Gavin Shan wrote:
> The emulation for EEH RTAS requests from guest isn't covered
> by QEMU yet and the patch implements them.
>
> The patch defines constants used by EEH RTAS calls and adds
> callback sPAPRPHBClass::eeh_handler, which is going to be used
> this way:
>
> 1. RTAS calls are received in spapr_pci.c, sanity check is done
> there.
> 2. RTAS handlers handle what they can. If there is something it
> cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
> it is called.
> 3. sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
> does ioctl() to the IOMMU container fd to complete the call. Error
> codes from that ioctl() are transferred back to the guest.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr_pci.c | 248 ++++++++++++++++++++++++++++++++++++++++++++
> include/hw/pci-host/spapr.h | 7 ++
> include/hw/ppc/spapr.h | 33 ++++++
> 3 files changed, 288 insertions(+)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 131434b..bfea488 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -422,6 +422,241 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
> }
>
> +static int rtas_handle_eeh_request(sPAPRPHBState *sphb,
> + uint32_t req, uint32_t opt,
> + target_ulong rets)
> +{
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> + int ret;
> +
> + ret = info->eeh_handler(sphb, req, opt);
> + if (ret >= 0) {
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + } else {
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> + }
> +
> + return ret;
I think you're better off failing the eeh_handler() call in here when
the function is not implemented. That way all callers don't have to
explicitly check for it.
I also find it a lot less confusing if the return value doesn't get set
inside this helper.
> +}
> +
> +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + uint32_t addr, option;
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 4) || (nret != 1)) {
> + goto param_error_exit;
> + }
> +
> + addr = rtas_ld(args, 0);
> + option = rtas_ld(args, 3);
> + switch (option) {
> + case RTAS_EEH_ENABLE:
> + if (!find_dev(spapr, buid, addr)) {
> + goto param_error_exit;
> + }
> + break;
> + case RTAS_EEH_DISABLE:
> + case RTAS_EEH_THAW_IO:
> + case RTAS_EEH_THAW_DMA:
> + break;
> + default:
> + goto param_error_exit;
> + }
> +
> + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_SET_OPTION, option, rets);
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + uint32_t addr, option;
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> + PCIDevice *pdev;
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 4) || (nret != 2)) {
> + goto param_error_exit;
> + }
> +
> + addr = rtas_ld(args, 0);
> + option = rtas_ld(args, 3);
> + if (option != RTAS_GET_PE_ADDR && option != RTAS_GET_PE_MODE) {
> + goto param_error_exit;
> + }
> +
> + pdev = find_dev(spapr, buid, addr);
> + if (!pdev) {
> + goto param_error_exit;
> + }
> +
> + /*
> + * For now, we always have bus level PE whose address
> + * has format "00BBSS00". The guest OS might regard
> + * PE address 0 as invalid. We avoid that simply by
> + * extending it with one.
> + */
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + if (option == RTAS_GET_PE_ADDR) {
> + rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1);
> + } else {
> + rtas_st(rets, 1, RTAS_PE_MODE_SHARED);
> + }
> +
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> +static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> + int32_t ret;
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 3) || (nret != 4 && nret != 5)) {
> + goto param_error_exit;
> + }
> +
> + ret = rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_GET_STATE, 0, rets);
> + if (ret >= 0) {
> + rtas_st(rets, 1, ret);
> + rtas_st(rets, 2, RTAS_EEH_SUPPORT);
> + rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO);
> + if (nret >= 5) {
> + rtas_st(rets, 4, RTAS_EEH_PE_RECOVER_INFO);
> + }
> + }
It's really awkward to not see an rtas_st(0) here.
> +
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> +static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + uint32_t option;
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 4) || (nret != 1)) {
> + goto param_error_exit;
> + }
> +
> + option = rtas_ld(args, 3);
> + if (option != RTAS_SLOT_RESET_DEACTIVATE &&
> + option != RTAS_SLOT_RESET_HOT &&
> + option != RTAS_SLOT_RESET_FUNDAMENTAL) {
> + goto param_error_exit;
> + }
Please make this a switch().
> +
> + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_RESET, option, rets);
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> +static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 3) || (nret != 1)) {
> + goto param_error_exit;
> + }
> +
> + rtas_handle_eeh_request(sphb, RTAS_EEH_REQ_CONFIGURE, 0, rets);
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> +/* To support it later */
> +static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + int option;
> + uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
> + sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +
> + if (!sphb || !info->eeh_handler) {
> + goto param_error_exit;
> + }
> +
> + if ((nargs != 8) || (nret != 1)) {
> + goto param_error_exit;
> + }
> +
> + option = rtas_ld(args, 7);
> + if (option != RTAS_SLOT_TEMP_ERR_LOG &&
> + option != RTAS_SLOT_PERM_ERR_LOG) {
> + goto param_error_exit;
> + }
switch() again.
Alex
> +
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + return;
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> static int pci_spapr_swizzle(int slot, int pin)
> {
> return (slot + pin) % PCI_NUM_PINS;
> @@ -939,6 +1174,19 @@ void spapr_pci_rtas_init(void)
> rtas_ibm_query_interrupt_source_number);
> spapr_rtas_register("ibm,change-msi", rtas_ibm_change_msi);
> }
> +
> + spapr_rtas_register("ibm,set-eeh-option",
> + rtas_ibm_set_eeh_option);
> + spapr_rtas_register("ibm,get-config-addr-info2",
> + rtas_ibm_get_config_addr_info2);
> + spapr_rtas_register("ibm,read-slot-reset-state2",
> + rtas_ibm_read_slot_reset_state2);
> + spapr_rtas_register("ibm,set-slot-reset",
> + rtas_ibm_set_slot_reset);
> + spapr_rtas_register("ibm,configure-pe",
> + rtas_ibm_configure_pe);
> + spapr_rtas_register("ibm,slot-error-detail",
> + rtas_ibm_slot_error_detail);
> }
>
> static void spapr_pci_register_types(void)
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index caeecfa..f487f23 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -51,6 +51,7 @@ struct sPAPRPHBClass {
> PCIHostBridgeClass parent_class;
>
> void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> + int (*eeh_handler)(sPAPRPHBState *sphb, int req, int opt);
> };
>
> struct sPAPRPHBState {
> @@ -104,6 +105,12 @@ struct sPAPRPHBVFIOState {
>
> #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
>
> +/* EEH related requests */
> +#define RTAS_EEH_REQ_SET_OPTION 0
> +#define RTAS_EEH_REQ_GET_STATE 1
> +#define RTAS_EEH_REQ_RESET 2
> +#define RTAS_EEH_REQ_CONFIGURE 3
> +
> static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
> {
> return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 6faae5d..ea84de2 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -354,6 +354,39 @@ static inline int spapr_allocate_lsi(int hint)
> return spapr_allocate_irq(hint, true);
> }
>
> +/* ibm,set-eeh-option */
> +#define RTAS_EEH_DISABLE 0
> +#define RTAS_EEH_ENABLE 1
> +#define RTAS_EEH_THAW_IO 2
> +#define RTAS_EEH_THAW_DMA 3
> +
> +/* ibm,get-config-addr-info2 */
> +#define RTAS_GET_PE_ADDR 0
> +#define RTAS_GET_PE_MODE 1
> +#define RTAS_PE_MODE_NONE 0
> +#define RTAS_PE_MODE_NOT_SHARED 1
> +#define RTAS_PE_MODE_SHARED 2
> +
> +/* ibm,read-slot-reset-state2 */
> +#define RTAS_EEH_PE_STATE_NORMAL 0
> +#define RTAS_EEH_PE_STATE_RESET 1
> +#define RTAS_EEH_PE_STATE_STOPPED_IO_DMA 2
> +#define RTAS_EEH_PE_STATE_STOPPED_DMA 4
> +#define RTAS_EEH_PE_STATE_UNAVAIL 5
> +#define RTAS_EEH_NOT_SUPPORT 0
> +#define RTAS_EEH_SUPPORT 1
> +#define RTAS_EEH_PE_UNAVAIL_INFO 1000
> +#define RTAS_EEH_PE_RECOVER_INFO 0
> +
> +/* ibm,set-slot-reset */
> +#define RTAS_SLOT_RESET_DEACTIVATE 0
> +#define RTAS_SLOT_RESET_HOT 1
> +#define RTAS_SLOT_RESET_FUNDAMENTAL 3
> +
> +/* ibm,slot-error-detail */
> +#define RTAS_SLOT_TEMP_ERR_LOG 1
> +#define RTAS_SLOT_PERM_ERR_LOG 2
> +
> /* RTAS return codes */
> #define RTAS_OUT_SUCCESS 0
> #define RTAS_OUT_NO_ERRORS_FOUND 1
next prev parent reply other threads:[~2014-06-24 14:43 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-10 2:03 [Qemu-devel] [PATCH v10 0/3] EEH Support for VFIO PCI Device Gavin Shan
2014-06-10 2:03 ` [Qemu-devel] [PATCH v10 1/3] sPAPR: Implement EEH RTAS calls Gavin Shan
2014-06-24 14:43 ` Alexander Graf [this message]
2014-06-24 23:38 ` Gavin Shan
2014-06-25 11:47 ` Alexander Graf
2014-06-10 2:03 ` [Qemu-devel] [PATCH v10 2/3] headers: Update kernel header Gavin Shan
2014-06-10 2:03 ` [Qemu-devel] [PATCH v10 3/3] sPAPR: Implement sPAPRPHBClass::eeh_handler Gavin Shan
2014-06-11 20:26 ` Alex Williamson
2014-06-12 0:02 ` Gavin Shan
2014-06-12 1:37 ` Alex Williamson
2014-06-16 1:24 ` Gavin Shan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53A98E8B.7010302@suse.de \
--to=agraf@suse.de \
--cc=aik@ozlabs.ru \
--cc=alex.williamson@redhat.com \
--cc=gwshan@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
--cc=qiudayu@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.