[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 12:15, Oleksii Kurochko wrote: > > 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? Yes, I do. Sorry for getting the terminology wrong. Jan > If yes, then an amount of them shouldn't be bigger > then 1 + BIT(guest_bits). > > ~ Oleksii >
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |