Re: [PATCH v2 10/16] xen/riscv: imsic_init() implementation

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/19/25 8:32 PM, Jan Beulich wrote:

> On 19.05.2025 17:19, Oleksii Kurochko wrote:
> > On 5/15/25 10:42 AM, Jan Beulich wrote:
> > > On 06.05.2025 18:51, Oleksii Kurochko wrote:
> > > > --- /dev/null
> > > > +++ b/xen/arch/riscv/include/asm/imsic.h
> > > > @@ -0,0 +1,65 @@
> > > > +/* SPDX-License-Identifier: MIT */
> > > > +
> > > > +/*
> > > > + * xen/arch/riscv/imsic.h
> > > > + *
> > > > + * RISC-V Incoming MSI Controller support
> > > > + *
> > > > + * (c) 2023 Microchip Technology Inc.
> > > > + */
> > > > +
> > > > +#ifndef ASM__RISCV__IMSIC_H
> > > > +#define ASM__RISCV__IMSIC_H
> > > > +
> > > > +#include <xen/types.h>
> > > > +
> > > > +#define IMSIC_MMIO_PAGE_SHIFT   12
> > > > +#define IMSIC_MMIO_PAGE_SZ      (1UL << IMSIC_MMIO_PAGE_SHIFT)
> > > > +
> > > > +#define IMSIC_MIN_ID            63
> > > >           
> > > This isn't the "minimum ID", but the "minimum permissible number of IDs" as per
> > > its first use in imsic_parse_node(). Further uses there consider it a mask,
> > > which makes me wonder whether the imsic_cfg.nr_ids field name is actually
> > > correct: Isn't this the maximum valid ID rather than their count/number?
> > >         
> > imsic_cfg.nr_ids it is a value of interrupt identities supported by IMSIC interrupt file according to
> > DT binding:
> >    riscv,num-ids:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >      minimum: 63
> >      maximum: 2047
> >      description:
> >        Number of interrupt identities supported by IMSIC interrupt file.
> >       
> Unless this count accounts for 0 being invalid (and hence isn't counted), I'm
> still confused: With the maximum value this would mean 2046 is still valid,
> but 2047 isn't anymore. When normally such a boundary would be at an exact
> power of 2, i.e. between 2047 and 2048 here.
>     
2047 is inclusive according to the AIA spec:
  The number of interrupt identities supported by an interrupt file
  (and hence the number of active bits in each array) is one less than a multiple
  of 64, and may be a minimum of 63 and a maximum of 2047.
  ...
  When an interrupt file supports N distinct interrupt identities,
  valid identity numbers are between 1 and N inclusive.
  The identity numbers within this range are said to be implemented by the interrupt
  file; numbers outside this range are not implemented.
  The number zero is never a valid interrupt identity. Identity 0 is just ignored.

It is still  not a power of two but it was the AIA spec tells us.
Also, this maximum identity number of 2047 is consistent with related fields like
the EIID (External Interrupt Identity) field used in APLICs when forwarding MSIs,
which specifies the MSI data value that becomes the minor identity at the target
hart's interrupt file.
The EIID field is typically an 11-bit field, able to hold values from 0 through
2047.
Since identity 0 is invalid, the entire range of valid identity numbers (1-2047)
fits within the values representable by an 11-bit field.

~ Oleksii