Re: [Xen-devel] [PATCH v5 3/7] xen/arm: zynqmp: introduce zynqmp specific defines

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 11 Dec 2018, Julien Grall wrote:
> Hi,
> 
> On 03/12/2018 21:03, Stefano Stabellini wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
> > 
> > From: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> > 
> > Introduce zynqmp specific defines for the firmware calls.
> > See EEMI:
> > https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
> > 
> > The error codes are described, under XIlPM Error Codes:
> > https://www.xilinx.com/support/documentation/user_guides/ug1137-zynq-ultrascale-mpsoc-swdev.pdf
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > 
> > ---
> > 
> > Although the amount of #defines has been significantly reduced in v5,
> > there is still a significant amount of static definitions:
> 
> I think the description below would be useful to have in the commit message.

I can add it.


> > - MM_*
> > These are the MMIO addresses of each resource to do permission checks.
> > Technically, they are also present on device tree, but using device tree
> > to get the mmio regions is not simple and not done in this series.
> > Xilinx plan to send a patch series in the future to extend the EEMI
> > functionalities and as part of that work, more device tree based
> > permission checking will be done, solving this problem.
> 
> As this is already present in the Device-Tree, there would be no issue to
> remove the hardcoded value, correct?

Yes, that is correct. Saeed took it on as part of his next patch series
to rework these checks, removing the MM_* #defines.


> > These
> > definitions could also be removed if we used a trivial "if dom0 -> yes,
> > otherwise no" permission checking.
> > 
> > - pm_api_id
> > These are the EEMI function IDs. Unavoidable.
> > 
> > - pm_ret_status
> > These are the EEMI return statuses. Unavoidable.
> > 
> > - pm_node_id
> > These are the EEMI function parameters for power management operations.
> > Today, it is not possible to get them from device tree as there is no
> > such information there. Even in the future when we add more power
> > domains info to device tree, the EEMI function parameters might remain
> > unique and different, requiring a table like this one.
> 
> Does it mean each Linux driver will have to hardcode the pm_node_id as well?

The EEMI functionalities upstream in Linux today don't need any node_id
knowledge: there is one ioctl that takes a node_id from userspace and
pass it to the firmware without any checks. There are few clock API
calls that take a clock id, but those are different from node_ids and
are discoverable via EEMI calls (they will be discoverable in Xen too,
patch series in development.) In other words, Linux doesn't do anything
with node_ids today.


> What is the plan there?

The plan is to extract the node_id from a power-domain node on device
tree. Each device would have a phandler to link to the right
power-domain node which contains a power-domain-id attribute. The
power-domain-id attribute is the node_id here.

The power-domain-id changes to the Xilinx MPSoC device tree are under
discussion with the device tree community.


> > - pm_reset
> > These are the EEMI function parameters for reset operations. Same as
> > pm_node_id.
> 
> Ditto.

Similar to above


> > 
> > ---
> > Changes in v5:
> > - remove MMIO access related definitions
> > 
> > Changes in v4:
> > - define PM_MMIO_SHIFT
> > ---
> >   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 293
> > +++++++++++++++++++++
> >   1 file changed, 293 insertions(+)
> > 
> > diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > index 43cefb5..f6ad03b 100644
> > --- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > @@ -16,6 +16,299 @@
> >     #include <asm/processor.h>
> >   +#define MM_RPU   0xff9a0
> > +#define MM_RTC     0xffa60
> > +#define MM_ADMA_CH0        0xffa80
> > +
> > +#define MM_USB3_0_XHCI  0xfe200
> > +#define MM_USB3_1_XHCI  0xfe300
> > +
> > +#define MM_SATA_AHCI_HBA   0xfd0c0
> > +#define MM_AXIPCIE_MAIN    0xfd0e0
> > +#define MM_CRF_APB 0xfd1a0000
> > +#define MM_PCIE_ATTRIB     0xfd480
> > +#define MM_DP      0xfd4a0
> > +#define MM_GPU     0xfd4b0
> > +#define MM_GDMA_CH0        0xfd500
> > +
> > +#define MM_UART0   0xff000
> > +#define MM_UART1   0xff010
> > +#define MM_I2C0    0xff020
> > +#define MM_I2C1    0xff030
> > +#define MM_SPI0    0xff040
> > +#define MM_SPI1    0xff050
> > +#define MM_CAN0    0xff060
> > +#define MM_CAN1    0xff070
> > +#define MM_GPIO    0xff0a0
> > +#define MM_GEM0    0xff0b0
> > +#define MM_GEM1    0xff0c0
> > +#define MM_GEM2    0xff0d0
> > +#define MM_GEM3    0xff0e0
> > +#define MM_QSPI    0xff0f0
> > +#define MM_NAND    0xff100
> > +#define MM_TTC0    0xff110
> > +#define MM_TTC1    0xff120
> > +#define MM_TTC2    0xff130
> > +#define MM_TTC3    0xff140
> > +#define MM_SWDT    0xff150
> > +#define MM_SD0     0xff160
> > +#define MM_SD1     0xff170
> > +
> > +/* Service calls.  */
> > +#define PM_GET_TRUSTZONE_VERSION   0xa03
> > +
> > +/* SMC function IDs for SiP Service queries */
> > +#define ZYNQMP_SIP_SVC_CALL_COUNT       0xff00
> > +#define ZYNQMP_SIP_SVC_UID              0xff01
> > +#define ZYNQMP_SIP_SVC_VERSION          0xff03
> 
> There are no need to define those. You can directly use ARM_SMCCC_CALL_* from
> asm-arm/smccc.h.
> 
> > +
> > +#define PM_MMIO_SHIFT                   32
> 
> You don't seem to use it at all. Did I miss anything?
> 
> > +
> > +enum pm_api_id {
> > +   /* Miscellaneous API functions: */
> > +   PM_GET_API_VERSION = 1, /* Do not change or move */
> > +   PM_SET_CONFIGURATION,
> > +   PM_GET_NODE_STATUS,
> > +   PM_GET_OP_CHARACTERISTIC,
> > +   PM_REGISTER_NOTIFIER,
> > +   /* API for suspending of PUs: */
> > +   PM_REQ_SUSPEND,
> > +   PM_SELF_SUSPEND,
> > +   PM_FORCE_POWERDOWN,
> > +   PM_ABORT_SUSPEND,
> > +   PM_REQ_WAKEUP,
> > +   PM_SET_WAKEUP_SOURCE,
> > +   PM_SYSTEM_SHUTDOWN,
> > +   /* API for managing PM slaves: */
> > +   PM_REQ_NODE,
> > +   PM_RELEASE_NODE,
> > +   PM_SET_REQUIREMENT,
> > +   PM_SET_MAX_LATENCY,
> > +   /* Direct control API functions: */
> > +   PM_RESET_ASSERT,
> > +   PM_RESET_GET_STATUS,
> > +   PM_MMIO_WRITE,
> > +   PM_MMIO_READ,
> > +   PM_INIT,
> > +   PM_FPGA_LOAD,
> > +   PM_FPGA_GET_STATUS,
> > +   PM_GET_CHIPID,
> > +   /* ID 25 is been used by U-boot to process secure boot images */
> > +   /* Secure library generic API functions */
> > +   PM_SECURE_SHA = 26,
> > +   PM_SECURE_RSA,
> > +   /* Pin control API functions */
> > +   PM_PINCTRL_REQUEST,
> > +   PM_PINCTRL_RELEASE,
> > +   PM_PINCTRL_GET_FUNCTION,
> > +   PM_PINCTRL_SET_FUNCTION,
> > +   PM_PINCTRL_CONFIG_PARAM_GET,
> > +   PM_PINCTRL_CONFIG_PARAM_SET,
> > +   /* PM IOCTL API */
> > +   PM_IOCTL,
> > +   /* API to query information from firmware */
> > +   PM_QUERY_DATA,
> > +   /* Clock control API functions */
> > +   PM_CLOCK_ENABLE,
> > +   PM_CLOCK_DISABLE,
> > +   PM_CLOCK_GETSTATE,
> > +   PM_CLOCK_SETDIVIDER,
> > +   PM_CLOCK_GETDIVIDER,
> > +   PM_CLOCK_SETRATE,
> > +   PM_CLOCK_GETRATE,
> > +   PM_CLOCK_SETPARENT,
> > +   PM_CLOCK_GETPARENT,
> > +   PM_API_MAX
> > +};
> > +
> > +enum pm_node_id {
> > +   NODE_RPU = 6,
> > +   NODE_RPU_0,
> > +   NODE_RPU_1,
> > +   NODE_GPU_PP_0 = 20,
> > +   NODE_GPU_PP_1,
> > +   NODE_USB_0,
> > +   NODE_USB_1,
> > +   NODE_TTC_0,
> > +   NODE_TTC_1,
> > +   NODE_TTC_2,
> > +   NODE_TTC_3,
> > +   NODE_SATA,
> > +   NODE_ETH_0,
> > +   NODE_ETH_1,
> > +   NODE_ETH_2,
> > +   NODE_ETH_3,
> > +   NODE_UART_0,
> > +   NODE_UART_1,
> > +   NODE_SPI_0,
> > +   NODE_SPI_1,
> > +   NODE_I2C_0,
> > +   NODE_I2C_1,
> > +   NODE_SD_0,
> > +   NODE_SD_1,
> > +   NODE_DP,
> > +   NODE_GDMA,
> > +   NODE_ADMA,
> > +   NODE_NAND,
> > +   NODE_QSPI,
> > +   NODE_GPIO,
> > +   NODE_CAN_0,
> > +   NODE_CAN_1,
> > +   NODE_AFI,
> > +   NODE_APLL,
> > +   NODE_VPLL,
> > +   NODE_DPLL,
> > +   NODE_RPLL,
> > +   NODE_IOPLL,
> > +   NODE_DDR,
> > +   NODE_IPI_APU,
> > +   NODE_IPI_RPU_0,
> > +   NODE_GPU,
> > +   NODE_PCIE,
> > +   NODE_PCAP,
> > +   NODE_RTC,
> > +};
> > +
> > +/**
> > + * @XST_PM_SUCCESS:                Success
> > + * @XST_PM_INTERNAL:       Unexpected error
> > + * @XST_PM_CONFLICT:       Conflicting requirements
> > + * @XST_PM_NO_ACCESS:      Access rights violation
> > + * @XST_PM_INVALID_NODE:   Does not apply to node passed as argument
> > + * @XST_PM_DOUBLE_REQ:     Duplicate request
> > + * @XST_PM_ABORT_SUSPEND:  Target has aborted suspend
> > + */
> > +enum pm_ret_status {
> > +   XST_PM_SUCCESS = 0,
> > +   XST_PM_INTERNAL = 2000,
> > +   XST_PM_CONFLICT,
> > +   XST_PM_NO_ACCESS,
> > +   XST_PM_INVALID_NODE,
> > +   XST_PM_DOUBLE_REQ,
> > +   XST_PM_ABORT_SUSPEND,
> > +};
> > +
> > +enum pm_reset {
> > +   XILPM_RESET_START = 999,
> > +   XILPM_RESET_PCIE_CFG,
> > +   XILPM_RESET_PCIE_BRIDGE,
> > +   XILPM_RESET_PCIE_CTRL,
> > +   XILPM_RESET_DP,
> > +   XILPM_RESET_SWDT_CRF,
> > +   XILPM_RESET_AFI_FM5,
> > +   XILPM_RESET_AFI_FM4,
> > +   XILPM_RESET_AFI_FM3,
> > +   XILPM_RESET_AFI_FM2,
> > +   XILPM_RESET_AFI_FM1,
> > +   XILPM_RESET_AFI_FM0,
> > +   XILPM_RESET_GDMA,
> > +   XILPM_RESET_GPU_PP1,
> > +   XILPM_RESET_GPU_PP0,
> > +   XILPM_RESET_GPU,
> > +   XILPM_RESET_GT,
> > +   XILPM_RESET_SATA,
> > +   XILPM_RESET_ACPU3_PWRON,
> > +   XILPM_RESET_ACPU2_PWRON,
> > +   XILPM_RESET_ACPU1_PWRON,
> > +   XILPM_RESET_ACPU0_PWRON,
> > +   XILPM_RESET_APU_L2,
> > +   XILPM_RESET_ACPU3,
> > +   XILPM_RESET_ACPU2,
> > +   XILPM_RESET_ACPU1,
> > +   XILPM_RESET_ACPU0,
> > +   XILPM_RESET_DDR,
> > +   XILPM_RESET_APM_FPD,
> > +   XILPM_RESET_SOFT,
> > +   XILPM_RESET_GEM0,
> > +   XILPM_RESET_GEM1,
> > +   XILPM_RESET_GEM2,
> > +   XILPM_RESET_GEM3,
> > +   XILPM_RESET_QSPI,
> > +   XILPM_RESET_UART0,
> > +   XILPM_RESET_UART1,
> > +   XILPM_RESET_SPI0,
> > +   XILPM_RESET_SPI1,
> > +   XILPM_RESET_SDIO0,
> > +   XILPM_RESET_SDIO1,
> > +   XILPM_RESET_CAN0,
> > +   XILPM_RESET_CAN1,
> > +   XILPM_RESET_I2C0,
> > +   XILPM_RESET_I2C1,
> > +   XILPM_RESET_TTC0,
> > +   XILPM_RESET_TTC1,
> > +   XILPM_RESET_TTC2,
> > +   XILPM_RESET_TTC3,
> > +   XILPM_RESET_SWDT_CRL,
> > +   XILPM_RESET_NAND,
> > +   XILPM_RESET_ADMA,
> > +   XILPM_RESET_GPIO,
> > +   XILPM_RESET_IOU_CC,
> > +   XILPM_RESET_TIMESTAMP,
> > +   XILPM_RESET_RPU_R50,
> > +   XILPM_RESET_RPU_R51,
> > +   XILPM_RESET_RPU_AMBA,
> > +   XILPM_RESET_OCM,
> > +   XILPM_RESET_RPU_PGE,
> > +   XILPM_RESET_USB0_CORERESET,
> > +   XILPM_RESET_USB1_CORERESET,
> > +   XILPM_RESET_USB0_HIBERRESET,
> > +   XILPM_RESET_USB1_HIBERRESET,
> > +   XILPM_RESET_USB0_APB,
> > +   XILPM_RESET_USB1_APB,
> > +   XILPM_RESET_IPI,
> > +   XILPM_RESET_APM_LPD,
> > +   XILPM_RESET_RTC,
> > +   XILPM_RESET_SYSMON,
> > +   XILPM_RESET_AFI_FM6,
> > +   XILPM_RESET_LPD_SWDT,
> > +   XILPM_RESET_FPD,
> > +   XILPM_RESET_RPU_DBG1,
> > +   XILPM_RESET_RPU_DBG0,
> > +   XILPM_RESET_DBG_LPD,
> > +   XILPM_RESET_DBG_FPD,
> > +   XILPM_RESET_APLL,
> > +   XILPM_RESET_DPLL,
> > +   XILPM_RESET_VPLL,
> > +   XILPM_RESET_IOPLL,
> > +   XILPM_RESET_RPLL,
> > +   XILPM_RESET_GPO3_PL_0,
> > +   XILPM_RESET_GPO3_PL_1,
> > +   XILPM_RESET_GPO3_PL_2,
> > +   XILPM_RESET_GPO3_PL_3,
> > +   XILPM_RESET_GPO3_PL_4,
> > +   XILPM_RESET_GPO3_PL_5,
> > +   XILPM_RESET_GPO3_PL_6,
> > +   XILPM_RESET_GPO3_PL_7,
> > +   XILPM_RESET_GPO3_PL_8,
> > +   XILPM_RESET_GPO3_PL_9,
> > +   XILPM_RESET_GPO3_PL_10,
> > +   XILPM_RESET_GPO3_PL_11,
> > +   XILPM_RESET_GPO3_PL_12,
> > +   XILPM_RESET_GPO3_PL_13,
> > +   XILPM_RESET_GPO3_PL_14,
> > +   XILPM_RESET_GPO3_PL_15,
> > +   XILPM_RESET_GPO3_PL_16,
> > +   XILPM_RESET_GPO3_PL_17,
> > +   XILPM_RESET_GPO3_PL_18,
> > +   XILPM_RESET_GPO3_PL_19,
> > +   XILPM_RESET_GPO3_PL_20,
> > +   XILPM_RESET_GPO3_PL_21,
> > +   XILPM_RESET_GPO3_PL_22,
> > +   XILPM_RESET_GPO3_PL_23,
> > +   XILPM_RESET_GPO3_PL_24,
> > +   XILPM_RESET_GPO3_PL_25,
> > +   XILPM_RESET_GPO3_PL_26,
> > +   XILPM_RESET_GPO3_PL_27,
> > +   XILPM_RESET_GPO3_PL_28,
> > +   XILPM_RESET_GPO3_PL_29,
> > +   XILPM_RESET_GPO3_PL_30,
> > +   XILPM_RESET_GPO3_PL_31,
> > +   XILPM_RESET_RPU_LS,
> > +   XILPM_RESET_PS_ONLY,
> > +   XILPM_RESET_PL,
> > +   XILPM_RESET_END
> > +};
> > +
> >   extern bool zynqmp_eemi(struct cpu_user_regs *regs);
> >     #endif /* __ASM_ARM_PLATFORMS_ZYNQMP_H */
> > 
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel