[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 08/14] xen/riscv: imsic_init() implementation
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. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |