[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 08/14] xen/riscv: imsic_init() implementation
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
- Date: Thu, 5 Jun 2025 12:15:51 +0200
- Cc: Alistair Francis <alistair.francis@xxxxxxx>, Bob Eshleman <bobbyeshleman@xxxxxxxxx>, Connor Davis <connojdavis@xxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Romain Caritey <Romain.Caritey@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Thu, 05 Jun 2025 10:16:00 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 6/5/25 11:42 AM, Jan Beulich wrote:
On 05.06.2025 11:13, Oleksii Kurochko wrote:
On 6/5/25 8:50 AM, Jan Beulich wrote:
On 04.06.2025 17:36, Oleksii Kurochko wrote:
On 6/2/25 12:21 PM, Jan Beulich wrote:
On 26.05.2025 20:44, Oleksii Kurochko wrote:
On 5/22/25 4:46 PM, Jan Beulich wrote:
On 21.05.2025 18:03, Oleksii Kurochko wrote:
+ /* Check MMIO register sets */
+ for ( unsigned int i = 0; i < nr_mmios; i++ )
+ {
+ if ( !alloc_cpumask_var(&imsic_cfg.mmios[i].cpus) )
+ {
+ rc = -ENOMEM;
+ goto imsic_init_err;
+ }
+
+ rc = dt_device_get_address(node, i, &imsic_cfg.mmios[i].base_addr,
+ &imsic_cfg.mmios[i].size);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "%s: unable to parse MMIO regset %u\n",
+ node->name, i);
+ goto imsic_init_err;
+ }
+
+ base_addr = imsic_cfg.mmios[i].base_addr;
+ base_addr &= ~(BIT(imsic_cfg.guest_index_bits +
+ imsic_cfg.hart_index_bits +
+ IMSIC_MMIO_PAGE_SHIFT, UL) - 1);
+ base_addr &= ~((BIT(imsic_cfg.group_index_bits, UL) - 1) <<
+ imsic_cfg.group_index_shift);
+ if ( base_addr != imsic_cfg.base_addr )
+ {
+ rc = -EINVAL;
+ printk(XENLOG_ERR "%s: address mismatch for regset %u\n",
+ node->name, i);
+ goto imsic_init_err;
+ }
Maybe just for my own understanding: There's no similar check for the
sizes to match / be consistent wanted / needed?
If you are speaking about imsic_cfg.mmios[i].size then it depends fully on h/w will
provide, IMO.
So I don't what is possible range for imsic_cfg.mmios[i].size.
Well, all I can say is that's it feels odd that you sanity check base_addr
but permit effectively any size.
Okay, I think I have two ideas how to check the size:
1. Based on guest bits from IMSIC's DT node. QEMU calculates a size as:
for (socket = 0; socket < socket_count; socket++) {
imsic_addr = base_addr + socket * VIRT_IMSIC_GROUP_MAX_SIZE;
imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) *
s->soc[socket].num_harts;
...
where:
#define IMSIC_MMIO_PAGE_SHIFT 12
#define IMSIC_MMIO_PAGE_SZ (1UL << IMSIC_MMIO_PAGE_SHIFT)
#define IMSIC_HART_NUM_GUESTS(__guest_bits) \
(1U << (__guest_bits))
#define IMSIC_HART_SIZE(__guest_bits) \
(IMSIC_HART_NUM_GUESTS(__guest_bits) * IMSIC_MMIO_PAGE_SZ)
2. Just take a theoretical maximum for S-mode IMSIC's node:
16,384 * 64 1(S-mode interrupt file) + 63(max guest interrupt files)) * 4 KiB
Where,
16,384 - maximum possible amount of harts according to AIA spec
64 - a maximum amount of possible interrupt file for S-mode IMSIC node:
1 - S interupt file + 63 guest interrupt files.
4 Kib - a maximum size of one interrupt file.
Which option is preferred?
I would have said 2, if your outline used "actual" rather than "maximum" values.
In option 2 maximum possible values are used. If we want to use "actual" values then
the option 1 should be used.
Actually I was wrong with request "actual" uniformly. It's only the hart count where
I meant to ask for that. For interrupts we should allow the maximum possible unless
we already know their count.
Do you mean 'interrupt file' here? If yes, then an amount of them shouldn't be bigger
then 1 + BIT(guest_bits).
~ Oleksii
|
