| Message ID | 6c6e7482cc3b0332f5724c80bf16931fe2d793ae.1744126720.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: introduce basic UART support and interrupts for hypervisor mode | expand |
On 08.04.2025 17:57, Oleksii Kurochko wrote: > @@ -21,6 +22,22 @@ static struct intc_info __ro_after_init aplic_info = { > .hw_version = INTC_APLIC, > }; > > +static int aplic_irq_xlate(const uint32_t *intspec, unsigned int intsize, As you start adding functions calling indirectly, please consider adding cf_check right away, even if right now this has no effect on RISC-V. That'll save you from going through the entire RISC-V subtree later on to find them all. > + unsigned int *out_hwirq, > + unsigned int *out_type) > +{ > + if ( intsize < 2 ) > + return -EINVAL; > + > + /* Mapping 1:1 */ > + *out_hwirq = intspec[0]; > + > + if ( out_type ) > + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK; > + > + return 0; > +} > + > static int __init aplic_preinit(struct dt_device_node *node, const void *dat) > { > if ( aplic_info.node ) > @@ -35,6 +52,8 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat) > > aplic_info.node = node; > > + dt_irq_xlate = aplic_irq_xlate; > + > return 0; > } > > --- a/xen/arch/riscv/include/asm/irq.h > +++ b/xen/arch/riscv/include/asm/irq.h > @@ -47,6 +47,9 @@ static inline void arch_move_irqs(struct vcpu *v) > BUG_ON("unimplemented"); > } > > +struct dt_device_node; > +int platform_get_irq(const struct dt_device_node *device, int index); And I assume callers of this will appear later in the series. > --- a/xen/arch/riscv/irq.c > +++ b/xen/arch/riscv/irq.c > @@ -7,11 +7,52 @@ > */ > > #include <xen/bug.h> > +#include <xen/device_tree.h> > +#include <xen/errno.h> > #include <xen/init.h> > #include <xen/irq.h> > > static irq_desc_t irq_desc[NR_IRQS]; > > +static bool irq_validate_new_type(unsigned int curr, unsigned int new) > +{ > + return (curr == IRQ_TYPE_INVALID || curr == new ); > +} > + > +static int irq_set_type(unsigned int irq, unsigned int type) > +{ > + unsigned long flags; > + struct irq_desc *desc = irq_to_desc(irq); > + int ret = -EBUSY; > + > + spin_lock_irqsave(&desc->lock, flags); > + > + if ( !irq_validate_new_type(desc->arch.type, type) ) > + goto err; > + > + desc->arch.type = type; > + > + ret = 0; > + > +err: Labels indented by at least one blank please. > + spin_unlock_irqrestore(&desc->lock, flags); > + > + return ret; > +} > + > +int platform_get_irq(const struct dt_device_node *device, int index) > +{ > + struct dt_irq dt_irq; > + > + if ( dt_device_get_irq(device, index, &dt_irq) ) > + return -1; > + > + if ( irq_set_type(dt_irq.irq, dt_irq.type) ) > + return -1; Can you please return proper -E... values, perhaps ones coming back from the functions called? Jan
On 4/10/25 5:35 PM, Jan Beulich wrote: > On 08.04.2025 17:57, Oleksii Kurochko wrote: >> @@ -21,6 +22,22 @@ static struct intc_info __ro_after_init aplic_info = { >> .hw_version = INTC_APLIC, >> }; >> >> +static int aplic_irq_xlate(const uint32_t *intspec, unsigned int intsize, > As you start adding functions calling indirectly, please consider adding cf_check > right away, even if right now this has no effect on RISC-V. That'll save you from > going through the entire RISC-V subtree later on to find them all. Sure. I thought that it is a feature for x86 as I haven't seen such attribute for Arm and RISC-V in GCC manuals. > >> + unsigned int *out_hwirq, >> + unsigned int *out_type) >> +{ >> + if ( intsize < 2 ) >> + return -EINVAL; >> + >> + /* Mapping 1:1 */ >> + *out_hwirq = intspec[0]; >> + >> + if ( out_type ) >> + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK; >> + >> + return 0; >> +} >> + >> static int __init aplic_preinit(struct dt_device_node *node, const void *dat) >> { >> if ( aplic_info.node ) >> @@ -35,6 +52,8 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat) >> >> aplic_info.node = node; >> >> + dt_irq_xlate = aplic_irq_xlate; >> + >> return 0; >> } >> >> --- a/xen/arch/riscv/include/asm/irq.h >> +++ b/xen/arch/riscv/include/asm/irq.h >> @@ -47,6 +47,9 @@ static inline void arch_move_irqs(struct vcpu *v) >> BUG_ON("unimplemented"); >> } >> >> +struct dt_device_node; >> +int platform_get_irq(const struct dt_device_node *device, int index); > And I assume callers of this will appear later in the series. Yes, it will be called ns16550_uart_dt_init() when CONFIG_NS16550 will be enabled for RISC-V. > >> --- a/xen/arch/riscv/irq.c >> +++ b/xen/arch/riscv/irq.c >> @@ -7,11 +7,52 @@ >> */ >> >> #include <xen/bug.h> >> +#include <xen/device_tree.h> >> +#include <xen/errno.h> >> #include <xen/init.h> >> #include <xen/irq.h> >> >> static irq_desc_t irq_desc[NR_IRQS]; >> >> +static bool irq_validate_new_type(unsigned int curr, unsigned int new) >> +{ >> + return (curr == IRQ_TYPE_INVALID || curr == new ); >> +} >> + >> +static int irq_set_type(unsigned int irq, unsigned int type) >> +{ >> + unsigned long flags; >> + struct irq_desc *desc = irq_to_desc(irq); >> + int ret = -EBUSY; >> + >> + spin_lock_irqsave(&desc->lock, flags); >> + >> + if ( !irq_validate_new_type(desc->arch.type, type) ) >> + goto err; >> + >> + desc->arch.type = type; >> + >> + ret = 0; >> + >> +err: > Labels indented by at least one blank please. > >> + spin_unlock_irqrestore(&desc->lock, flags); >> + >> + return ret; >> +} >> + >> +int platform_get_irq(const struct dt_device_node *device, int index) >> +{ >> + struct dt_irq dt_irq; >> + >> + if ( dt_device_get_irq(device, index, &dt_irq) ) >> + return -1; >> + >> + if ( irq_set_type(dt_irq.irq, dt_irq.type) ) >> + return -1; > Can you please return proper -E... values, perhaps ones coming back from the > functions called? Sure, I will use -EINVAL. (or ,perhaps, it will be better to introduce ret and return what dt_device_get_irq()/irq_set_type() returns in the case of failure. ~ Oleksii
On 15.04.2025 13:11, Oleksii Kurochko wrote: > On 4/10/25 5:35 PM, Jan Beulich wrote: >> On 08.04.2025 17:57, Oleksii Kurochko wrote: >>> @@ -21,6 +22,22 @@ static struct intc_info __ro_after_init aplic_info = { >>> .hw_version = INTC_APLIC, >>> }; >>> >>> +static int aplic_irq_xlate(const uint32_t *intspec, unsigned int intsize, >> As you start adding functions calling indirectly, please consider adding cf_check >> right away, even if right now this has no effect on RISC-V. That'll save you from >> going through the entire RISC-V subtree later on to find them all. > > Sure. I thought that it is a feature for x86 as I haven't seen such attribute for > Arm and RISC-V in GCC manuals. And that looks to be correct. I was under the (admittedly vague) impression Arm64 had something equivalent in hardware, which then merely needs enabling in the compiler. Not sure about RISC-V, but seeing the endless flow of patches enabling new extensions in binutils, it would perhaps even be surprising if nothing along these lines was already in the works somewhere. Jan
On 4/15/25 1:23 PM, Jan Beulich wrote: > On 15.04.2025 13:11, Oleksii Kurochko wrote: >> On 4/10/25 5:35 PM, Jan Beulich wrote: >>> On 08.04.2025 17:57, Oleksii Kurochko wrote: >>>> @@ -21,6 +22,22 @@ static struct intc_info __ro_after_init aplic_info = { >>>> .hw_version = INTC_APLIC, >>>> }; >>>> >>>> +static int aplic_irq_xlate(const uint32_t *intspec, unsigned int intsize, >>> As you start adding functions calling indirectly, please consider adding cf_check >>> right away, even if right now this has no effect on RISC-V. That'll save you from >>> going through the entire RISC-V subtree later on to find them all. >> Sure. I thought that it is a feature for x86 as I haven't seen such attribute for >> Arm and RISC-V in GCC manuals. > And that looks to be correct. I was under the (admittedly vague) impression > Arm64 had something equivalent in hardware, which then merely needs enabling > in the compiler. Not sure about RISC-V, but seeing the endless flow of > patches enabling new extensions in binutils, it would perhaps even be > surprising if nothing along these lines was already in the works somewhere. You are right, something is already in the work: -https://github.com/riscv/riscv-cfi -https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/ (interesting that they are enabling it for U-mode) ~ Oleksii
diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c index caba8f8993..6dc040af6f 100644 --- a/xen/arch/riscv/aplic.c +++ b/xen/arch/riscv/aplic.c @@ -11,6 +11,7 @@ #include <xen/errno.h> #include <xen/init.h> +#include <xen/irq.h> #include <xen/sections.h> #include <xen/types.h> @@ -21,6 +22,22 @@ static struct intc_info __ro_after_init aplic_info = { .hw_version = INTC_APLIC, }; +static int aplic_irq_xlate(const uint32_t *intspec, unsigned int intsize, + unsigned int *out_hwirq, + unsigned int *out_type) +{ + if ( intsize < 2 ) + return -EINVAL; + + /* Mapping 1:1 */ + *out_hwirq = intspec[0]; + + if ( out_type ) + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK; + + return 0; +} + static int __init aplic_preinit(struct dt_device_node *node, const void *dat) { if ( aplic_info.node ) @@ -35,6 +52,8 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat) aplic_info.node = node; + dt_irq_xlate = aplic_irq_xlate; + return 0; } diff --git a/xen/arch/riscv/include/asm/irq.h b/xen/arch/riscv/include/asm/irq.h index 8f936b7d01..ff1c95e0be 100644 --- a/xen/arch/riscv/include/asm/irq.h +++ b/xen/arch/riscv/include/asm/irq.h @@ -47,6 +47,9 @@ static inline void arch_move_irqs(struct vcpu *v) BUG_ON("unimplemented"); } +struct dt_device_node; +int platform_get_irq(const struct dt_device_node *device, int index); + void init_IRQ(void); #endif /* ASM__RISCV__IRQ_H */ diff --git a/xen/arch/riscv/irq.c b/xen/arch/riscv/irq.c index 99b8f2095e..c332e000c4 100644 --- a/xen/arch/riscv/irq.c +++ b/xen/arch/riscv/irq.c @@ -7,11 +7,52 @@ */ #include <xen/bug.h> +#include <xen/device_tree.h> +#include <xen/errno.h> #include <xen/init.h> #include <xen/irq.h> static irq_desc_t irq_desc[NR_IRQS]; +static bool irq_validate_new_type(unsigned int curr, unsigned int new) +{ + return (curr == IRQ_TYPE_INVALID || curr == new ); +} + +static int irq_set_type(unsigned int irq, unsigned int type) +{ + unsigned long flags; + struct irq_desc *desc = irq_to_desc(irq); + int ret = -EBUSY; + + spin_lock_irqsave(&desc->lock, flags); + + if ( !irq_validate_new_type(desc->arch.type, type) ) + goto err; + + desc->arch.type = type; + + ret = 0; + +err: + spin_unlock_irqrestore(&desc->lock, flags); + + return ret; +} + +int platform_get_irq(const struct dt_device_node *device, int index) +{ + struct dt_irq dt_irq; + + if ( dt_device_get_irq(device, index, &dt_irq) ) + return -1; + + if ( irq_set_type(dt_irq.irq, dt_irq.type) ) + return -1; + + return dt_irq.irq; +} + int arch_init_one_irq_desc(struct irq_desc *desc) { desc->arch.type = IRQ_TYPE_INVALID;
platform_get_irq() recieves information about device's irq ( type and irq number ) from device tree node and using this information update irq descriptor in irq_desc[] array. Introduce dt_irq_xlate and initialize with aplic_irq_xlate() as it is used by dt_device_get_irq() which is called by platform_get_irq(). Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/aplic.c | 19 +++++++++++++++ xen/arch/riscv/include/asm/irq.h | 3 +++ xen/arch/riscv/irq.c | 41 ++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+)