Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 05.06.2024 12:22, Chen, Jiqian wrote: > On 2024/6/5 18:09, Jan Beulich wrote: >> On 05.06.2024 09:04, Chen, Jiqian wrote: >>> For now, if hypervisor gets a high GSIs, it can't be transformed to irq, >>> because there is no mapping between them. >> >> No, in the absence of a source override (note the word "override") the >> default identity mapping applies. > What is identity mapping? GSI == IRQ Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/6/5 18:09, Jan Beulich wrote: > On 05.06.2024 09:04, Chen, Jiqian wrote: >> On 2024/6/5 01:17, Jan Beulich wrote: >>> On 04.06.2024 10:18, Chen, Jiqian wrote: I tried to get more debug information from my environment. And I attach them here, maybe you can find some problems. acpi_parse_madt_ioapic_entries acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, acpi_parse_int_src_ovr, MAX_IRQ_SOURCES); acpi_parse_int_src_ovr mp_override_legacy_irq only process two entries, irq 0 gsi 2 and irq 9 gsi 9 There are only two entries whose type is ACPI_MADT_TYPE_INTERRUPT_OVERRIDE in MADT table. Is it normal? >>> >>> Yes, that's what you'd typically see (or just one such entry). >> Ok, let me conclude that acpi_parse_int_src_ovr get two entries from MADT >> table and add them into mp_irqs. They are [irq, gsi][0, 2] and [irq, gsi][9, >> 9]. >> Then in the following function mp_config_acpi_legacy_irqs initializes the >> 1:1 mapping of irq and gsi [0~15 except 2 and 9], and add them into mp_irqs. >> But for high GSIs(>= 16), no mapping processing. >> Right? > > On that specific system of yours - yes. In the general case high GSIs > may have entries, too. > >> Is it that the Xen hypervisor lacks some handling of high GSIs? > > I don't think so. Unless you can point out something? Ok, so the implementation is still to get mapping from mp_irqs, I will change in next version. Thank you. > >> For now, if hypervisor gets a high GSIs, it can't be transformed to irq, >> because there is no mapping between them. > > No, in the absence of a source override (note the word "override") the > default identity mapping applies. What is identity mapping? Like the mp_config_acpi_legacy_irqs does? > > Jan -- Best regards, Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 05.06.2024 09:04, Chen, Jiqian wrote: > On 2024/6/5 01:17, Jan Beulich wrote: >> On 04.06.2024 10:18, Chen, Jiqian wrote: >>> I tried to get more debug information from my environment. And I attach >>> them here, maybe you can find some problems. >>> acpi_parse_madt_ioapic_entries >>> acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, >>> acpi_parse_int_src_ovr, MAX_IRQ_SOURCES); >>> acpi_parse_int_src_ovr >>> mp_override_legacy_irq >>> only process two entries, irq 0 gsi 2 and irq 9 >>> gsi 9 >>> There are only two entries whose type is ACPI_MADT_TYPE_INTERRUPT_OVERRIDE >>> in MADT table. Is it normal? >> >> Yes, that's what you'd typically see (or just one such entry). > Ok, let me conclude that acpi_parse_int_src_ovr get two entries from MADT > table and add them into mp_irqs. They are [irq, gsi][0, 2] and [irq, gsi][9, > 9]. > Then in the following function mp_config_acpi_legacy_irqs initializes the 1:1 > mapping of irq and gsi [0~15 except 2 and 9], and add them into mp_irqs. > But for high GSIs(>= 16), no mapping processing. > Right? On that specific system of yours - yes. In the general case high GSIs may have entries, too. > Is it that the Xen hypervisor lacks some handling of high GSIs? I don't think so. Unless you can point out something? > For now, if hypervisor gets a high GSIs, it can't be transformed to irq, > because there is no mapping between them. No, in the absence of a source override (note the word "override") the default identity mapping applies. Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/6/5 01:17, Jan Beulich wrote: > On 04.06.2024 10:18, Chen, Jiqian wrote: >> I tried to get more debug information from my environment. And I attach them >> here, maybe you can find some problems. >> acpi_parse_madt_ioapic_entries >> acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, >> acpi_parse_int_src_ovr, MAX_IRQ_SOURCES); >> acpi_parse_int_src_ovr >> mp_override_legacy_irq >> only process two entries, irq 0 gsi 2 and irq 9 >> gsi 9 >> There are only two entries whose type is ACPI_MADT_TYPE_INTERRUPT_OVERRIDE >> in MADT table. Is it normal? > > Yes, that's what you'd typically see (or just one such entry). Ok, let me conclude that acpi_parse_int_src_ovr get two entries from MADT table and add them into mp_irqs. They are [irq, gsi][0, 2] and [irq, gsi][9, 9]. Then in the following function mp_config_acpi_legacy_irqs initializes the 1:1 mapping of irq and gsi [0~15 except 2 and 9], and add them into mp_irqs. But for high GSIs(>= 16), no mapping processing. Right? Is it that the Xen hypervisor lacks some handling of high GSIs? For now, if hypervisor gets a high GSIs, it can't be transformed to irq, because there is no mapping between them. > > Jan -- Best regards, Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 04.06.2024 10:18, Chen, Jiqian wrote: > I tried to get more debug information from my environment. And I attach them > here, maybe you can find some problems. > acpi_parse_madt_ioapic_entries > acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, > acpi_parse_int_src_ovr, MAX_IRQ_SOURCES); > acpi_parse_int_src_ovr > mp_override_legacy_irq > only process two entries, irq 0 gsi 2 and irq 9 > gsi 9 > There are only two entries whose type is ACPI_MADT_TYPE_INTERRUPT_OVERRIDE in > MADT table. Is it normal? Yes, that's what you'd typically see (or just one such entry). Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/6/4 14:36, Jan Beulich wrote: > On 04.06.2024 08:33, Chen, Jiqian wrote: >> On 2024/6/4 14:12, Jan Beulich wrote: >>> On 04.06.2024 08:01, Chen, Jiqian wrote: On 2024/6/4 13:55, Jan Beulich wrote: > On 04.06.2024 05:04, Chen, Jiqian wrote: >> On 2024/5/30 23:51, Jan Beulich wrote: >>> On 30.05.2024 13:19, Chen, Jiqian wrote: It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs. Other gsi can be considered 1:1 mapping with irq? Or are there other places reflect the mapping between irq and gsi? >>> >>> It may be uncommon to have overrides for higher GSIs, but I don't think >>> ACPI >>> disallows that. >> Do you suggest me to add overrides for higher GSIs into array mp_irqs? > > Why "add"? That's what mp_override_legacy_irq() already does, isn't it? No. mp_override_legacy_irq only overrides for gsi < 16, but not for gsi >= 16(I dump all mappings from array mp_irqs). >>> >>> I assume you mean you observe so ... >> No, after starting xen pvh dom0, I dump all entries from mp_irqs. > > IOW really your answer is "yes" ... > In my environment, gsi of my dGPU is 24. >>> >>> ... on one specific system? > > ... to this question I raised. Whatever you dump on any number of > systems, there's always the chance that there's another system > where things are different. > >>> The function is invoked from >>> acpi_parse_int_src_ovr(), and I can't spot any restriction to >>> IRQs less than 16 there. >> I didn't see any restriction too, but from the dump results, there are only >> 16 entries, see previous email. > > Hence why I tried to point out that going from observations on a > particular system isn't enough. Anyway, I agree with you that I need to get mapping from mp_irqs. I tried to get more debug information from my environment. And I attach them here, maybe you can find some problems. acpi_parse_madt_ioapic_entries acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, acpi_parse_int_src_ovr, MAX_IRQ_SOURCES); acpi_parse_int_src_ovr mp_override_legacy_irq only process two entries, irq 0 gsi 2 and irq 9 gsi 9 There are only two entries whose type is ACPI_MADT_TYPE_INTERRUPT_OVERRIDE in MADT table. Is it normal? And acpi_parse_madt_ioapic_entries mp_config_acpi_legacy_irqs process the other GSIs(< 16), so that the total number of mp_irqs is 16. > > Jan -- Best regards, Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 04.06.2024 08:33, Chen, Jiqian wrote: > On 2024/6/4 14:12, Jan Beulich wrote: >> On 04.06.2024 08:01, Chen, Jiqian wrote: >>> On 2024/6/4 13:55, Jan Beulich wrote: On 04.06.2024 05:04, Chen, Jiqian wrote: > On 2024/5/30 23:51, Jan Beulich wrote: >> On 30.05.2024 13:19, Chen, Jiqian wrote: >>> It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs. >>> Other gsi can be considered 1:1 mapping with irq? Or are there other >>> places reflect the mapping between irq and gsi? >> >> It may be uncommon to have overrides for higher GSIs, but I don't think >> ACPI >> disallows that. > Do you suggest me to add overrides for higher GSIs into array mp_irqs? Why "add"? That's what mp_override_legacy_irq() already does, isn't it? >>> No. mp_override_legacy_irq only overrides for gsi < 16, but not for gsi >= >>> 16(I dump all mappings from array mp_irqs). >> >> I assume you mean you observe so ... > No, after starting xen pvh dom0, I dump all entries from mp_irqs. IOW really your answer is "yes" ... >>> In my environment, gsi of my dGPU is 24. >> >> ... on one specific system? ... to this question I raised. Whatever you dump on any number of systems, there's always the chance that there's another system where things are different. >> The function is invoked from >> acpi_parse_int_src_ovr(), and I can't spot any restriction to >> IRQs less than 16 there. > I didn't see any restriction too, but from the dump results, there are only > 16 entries, see previous email. Hence why I tried to point out that going from observations on a particular system isn't enough. Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/6/4 14:12, Jan Beulich wrote: > On 04.06.2024 08:01, Chen, Jiqian wrote: >> On 2024/6/4 13:55, Jan Beulich wrote: >>> On 04.06.2024 05:04, Chen, Jiqian wrote: On 2024/5/30 23:51, Jan Beulich wrote: > On 30.05.2024 13:19, Chen, Jiqian wrote: >> I dump all mpc_config_intsrc of array mp_irqs, it shows: >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0 >> dstapic 33 dstirq 2 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9 >> dstapic 33 dstirq 9 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1 >> dstapic 33 dstirq 1 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3 >> dstapic 33 dstirq 3 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4 >> dstapic 33 dstirq 4 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5 >> dstapic 33 dstirq 5 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6 >> dstapic 33 dstirq 6 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7 >> dstapic 33 dstirq 7 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8 >> dstapic 33 dstirq 8 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10 >> dstapic 33 dstirq 10 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11 >> dstapic 33 dstirq 11 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12 >> dstapic 33 dstirq 12 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13 >> dstapic 33 dstirq 13 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14 >> dstapic 33 dstirq 14 >> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15 >> dstapic 33 dstirq 15 >> >> It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs. >> Other gsi can be considered 1:1 mapping with irq? Or are there other >> places reflect the mapping between irq and gsi? > > It may be uncommon to have overrides for higher GSIs, but I don't think > ACPI > disallows that. Do you suggest me to add overrides for higher GSIs into array mp_irqs? >>> >>> Why "add"? That's what mp_override_legacy_irq() already does, isn't it? >> No. mp_override_legacy_irq only overrides for gsi < 16, but not for gsi >= >> 16(I dump all mappings from array mp_irqs). > > I assume you mean you observe so ... No, after starting xen pvh dom0, I dump all entries from mp_irqs. > >> In my environment, gsi of my dGPU is 24. > > ... on one specific system? The function is invoked from > acpi_parse_int_src_ovr(), and I can't spot any restriction to > IRQs less than 16 there. I didn't see any restriction too, but from the dump results, there are only 16 entries, see previous email. > > Jan -- Best regards, Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 04.06.2024 08:01, Chen, Jiqian wrote: > So, how do I process for gsi >= 16? Oh, and to answer this as well: Isn't it as simple as treating as 1:1 mapped any (valid) GSI you can't find in mp_irqs[]? You only care about the mapping, not e.g. polarity or trigger mode here, iirc. Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 04.06.2024 08:01, Chen, Jiqian wrote: > On 2024/6/4 13:55, Jan Beulich wrote: >> On 04.06.2024 05:04, Chen, Jiqian wrote: >>> On 2024/5/30 23:51, Jan Beulich wrote: On 30.05.2024 13:19, Chen, Jiqian wrote: > I dump all mpc_config_intsrc of array mp_irqs, it shows: > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0 > dstapic 33 dstirq 2 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9 > dstapic 33 dstirq 9 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1 > dstapic 33 dstirq 1 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3 > dstapic 33 dstirq 3 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4 > dstapic 33 dstirq 4 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5 > dstapic 33 dstirq 5 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6 > dstapic 33 dstirq 6 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7 > dstapic 33 dstirq 7 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8 > dstapic 33 dstirq 8 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10 > dstapic 33 dstirq 10 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11 > dstapic 33 dstirq 11 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12 > dstapic 33 dstirq 12 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13 > dstapic 33 dstirq 13 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14 > dstapic 33 dstirq 14 > (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15 > dstapic 33 dstirq 15 > > It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs. > Other gsi can be considered 1:1 mapping with irq? Or are there other > places reflect the mapping between irq and gsi? It may be uncommon to have overrides for higher GSIs, but I don't think ACPI disallows that. >>> Do you suggest me to add overrides for higher GSIs into array mp_irqs? >> >> Why "add"? That's what mp_override_legacy_irq() already does, isn't it? > No. mp_override_legacy_irq only overrides for gsi < 16, but not for gsi >= > 16(I dump all mappings from array mp_irqs). I assume you mean you observe so ... > In my environment, gsi of my dGPU is 24. ... on one specific system? The function is invoked from acpi_parse_int_src_ovr(), and I can't spot any restriction to IRQs less than 16 there. Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/6/4 13:55, Jan Beulich wrote:
> On 04.06.2024 05:04, Chen, Jiqian wrote:
>> On 2024/5/30 23:51, Jan Beulich wrote:
>>> On 30.05.2024 13:19, Chen, Jiqian wrote:
On 2024/5/29 20:22, Jan Beulich wrote:
> On 29.05.2024 13:13, Chen, Jiqian wrote:
>> On 2024/5/29 15:10, Jan Beulich wrote:
>>> On 29.05.2024 08:56, Chen, Jiqian wrote:
On 2024/5/29 14:31, Jan Beulich wrote:
> On 29.05.2024 04:41, Chen, Jiqian wrote:
>> But I found in function init_irq_data:
>> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>> {
>> int rc;
>>
>> desc = irq_to_desc(irq);
>> desc->irq = irq;
>>
>> rc = init_one_irq_desc(desc);
>> if ( rc )
>> return rc;
>> }
>> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
>> mapping?
>
> No, as explained before. I also don't see how you would derive that
> from the code above.
Because here set desc->irq = irq, and it seems there is no other place
to change this desc->irq, so, gsi 1 is considered to irq 1.
>>>
>>> What are you taking this from? The loop bound isn't nr_gsis, and the
>>> iteration
>>> variable isn't in GSI space either; it's in IRQ numbering space. In
>>> this loop
>>> we're merely leveraging that every GSI has a corresponding IRQ;
>>> there are no assumptions made about the mapping between the two. Afaics
>>> at least.
>>>
> "nr_irqs_gsi" describes what its name says: The number of
> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI
> (i.e.
> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
> mapping.
>
>> What's more, when using PHYSDEVOP_setup_gsi, it calls
>> mp_register_gsi,
>> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
>> irq_desc directly.
>
> Which may be wrong, while that wrong-ness may not have hit anyone in
> practice (for reasons that would need working out).
>
>> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi
>> ?
>
> Again - no.
Since you are certain that they are not equal, could you tell me where
show they are not equal or where build their mappings,
so that I can know how to do a conversion gsi from irq.
>>>
>>> I did point you at the ACPI Interrupt Source Override structure before.
>>> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
>>> start going from.
>> Oh! I think I know.
>> If I want to transform gsi to irq, I need to do below:
>> int irq, entry, ioapic, pin;
>>
>> ioapic = mp_find_ioapic(gsi);
>> pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
>> entry = find_irq_entry(ioapic, pin, mp_INT);
>> irq = pin_2_irq(entry, ioapic, pin);
>>
>> Am I right?
>
> This looks plausible, yes.
I dump all mpc_config_intsrc of array mp_irqs, it shows:
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0
dstapic 33 dstirq 2
(XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9
dstapic 33 dstirq 9
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1
dstapic 33 dstirq 1
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3
dstapic 33 dstirq 3
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4
dstapic 33 dstirq 4
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5
dstapic 33 dstirq 5
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6
dstapic 33 dstirq 6
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7
dstapic 33 dstirq 7
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8
dstapic 33 dstirq 8
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10
dstapic 33 dstirq 10
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11
dstapic 33 dstirq 11
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12
dstapic 33 dstirq 12
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13
dstapic 33 dstirq 13
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14
dstapic 33 dstirq 14
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15
dstapic 33 dstirq 15
It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs.
Other gsi can be considered 1:1 mapping with irq? Or are there other
places reflect the mapping between irq and gsi?
>>>
>>> It may be uncommon to have overrides for higher GSIs, but I don't think
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 04.06.2024 05:04, Chen, Jiqian wrote:
> On 2024/5/30 23:51, Jan Beulich wrote:
>> On 30.05.2024 13:19, Chen, Jiqian wrote:
>>> On 2024/5/29 20:22, Jan Beulich wrote:
On 29.05.2024 13:13, Chen, Jiqian wrote:
> On 2024/5/29 15:10, Jan Beulich wrote:
>> On 29.05.2024 08:56, Chen, Jiqian wrote:
>>> On 2024/5/29 14:31, Jan Beulich wrote:
On 29.05.2024 04:41, Chen, Jiqian wrote:
> But I found in function init_irq_data:
> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
> {
> int rc;
>
> desc = irq_to_desc(irq);
> desc->irq = irq;
>
> rc = init_one_irq_desc(desc);
> if ( rc )
> return rc;
> }
> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
> mapping?
No, as explained before. I also don't see how you would derive that
from the code above.
>>> Because here set desc->irq = irq, and it seems there is no other place
>>> to change this desc->irq, so, gsi 1 is considered to irq 1.
>>
>> What are you taking this from? The loop bound isn't nr_gsis, and the
>> iteration
>> variable isn't in GSI space either; it's in IRQ numbering space. In this
>> loop
>> we're merely leveraging that every GSI has a corresponding IRQ;
>> there are no assumptions made about the mapping between the two. Afaics
>> at least.
>>
"nr_irqs_gsi" describes what its name says: The number of
IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI
(i.e.
mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
mapping.
> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
> irq_desc directly.
Which may be wrong, while that wrong-ness may not have hit anyone in
practice (for reasons that would need working out).
> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
Again - no.
>>> Since you are certain that they are not equal, could you tell me where
>>> show they are not equal or where build their mappings,
>>> so that I can know how to do a conversion gsi from irq.
>>
>> I did point you at the ACPI Interrupt Source Override structure before.
>> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
>> start going from.
> Oh! I think I know.
> If I want to transform gsi to irq, I need to do below:
> int irq, entry, ioapic, pin;
>
> ioapic = mp_find_ioapic(gsi);
> pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
> entry = find_irq_entry(ioapic, pin, mp_INT);
> irq = pin_2_irq(entry, ioapic, pin);
>
> Am I right?
This looks plausible, yes.
>>> I dump all mpc_config_intsrc of array mp_irqs, it shows:
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0
>>> dstapic 33 dstirq 2
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9
>>> dstapic 33 dstirq 9
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1
>>> dstapic 33 dstirq 1
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3
>>> dstapic 33 dstirq 3
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4
>>> dstapic 33 dstirq 4
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5
>>> dstapic 33 dstirq 5
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6
>>> dstapic 33 dstirq 6
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7
>>> dstapic 33 dstirq 7
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8
>>> dstapic 33 dstirq 8
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10
>>> dstapic 33 dstirq 10
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11
>>> dstapic 33 dstirq 11
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12
>>> dstapic 33 dstirq 12
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13
>>> dstapic 33 dstirq 13
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14
>>> dstapic 33 dstirq 14
>>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15
>>> dstapic 33 dstirq 15
>>>
>>> It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs.
>>> Other gsi can be considered 1:1 mapping with irq? Or are there other places
>>> reflect the mapping between irq and gsi?
>>
>> It may be uncommon to have overrides for higher GSIs, but I don't think ACPI
>> disallows that.
> Do you suggest me to add overrides for higher GSIs into array mp_irqs?
Why "add"? That's what mp_override_legacy_irq() already does, isn't
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/30 23:51, Jan Beulich wrote:
> On 30.05.2024 13:19, Chen, Jiqian wrote:
>> On 2024/5/29 20:22, Jan Beulich wrote:
>>> On 29.05.2024 13:13, Chen, Jiqian wrote:
On 2024/5/29 15:10, Jan Beulich wrote:
> On 29.05.2024 08:56, Chen, Jiqian wrote:
>> On 2024/5/29 14:31, Jan Beulich wrote:
>>> On 29.05.2024 04:41, Chen, Jiqian wrote:
But I found in function init_irq_data:
for ( irq = 0; irq < nr_irqs_gsi; irq++ )
{
int rc;
desc = irq_to_desc(irq);
desc->irq = irq;
rc = init_one_irq_desc(desc);
if ( rc )
return rc;
}
Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
mapping?
>>>
>>> No, as explained before. I also don't see how you would derive that
>>> from the code above.
>> Because here set desc->irq = irq, and it seems there is no other place
>> to change this desc->irq, so, gsi 1 is considered to irq 1.
>
> What are you taking this from? The loop bound isn't nr_gsis, and the
> iteration
> variable isn't in GSI space either; it's in IRQ numbering space. In this
> loop
> we're merely leveraging that every GSI has a corresponding IRQ;
> there are no assumptions made about the mapping between the two. Afaics
> at least.
>
>>> "nr_irqs_gsi" describes what its name says: The number of
>>> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI
>>> (i.e.
>>> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
>>> mapping.
>>>
What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
irq_desc directly.
>>>
>>> Which may be wrong, while that wrong-ness may not have hit anyone in
>>> practice (for reasons that would need working out).
>>>
Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>>>
>>> Again - no.
>> Since you are certain that they are not equal, could you tell me where
>> show they are not equal or where build their mappings,
>> so that I can know how to do a conversion gsi from irq.
>
> I did point you at the ACPI Interrupt Source Override structure before.
> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
> start going from.
Oh! I think I know.
If I want to transform gsi to irq, I need to do below:
int irq, entry, ioapic, pin;
ioapic = mp_find_ioapic(gsi);
pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
entry = find_irq_entry(ioapic, pin, mp_INT);
irq = pin_2_irq(entry, ioapic, pin);
Am I right?
>>>
>>> This looks plausible, yes.
>> I dump all mpc_config_intsrc of array mp_irqs, it shows:
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0 dstapic
>> 33 dstirq 2
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9
>> dstapic 33 dstirq 9
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1 dstapic
>> 33 dstirq 1
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3 dstapic
>> 33 dstirq 3
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4 dstapic
>> 33 dstirq 4
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5 dstapic
>> 33 dstirq 5
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6 dstapic
>> 33 dstirq 6
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7 dstapic
>> 33 dstirq 7
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8 dstapic
>> 33 dstirq 8
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10
>> dstapic 33 dstirq 10
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11
>> dstapic 33 dstirq 11
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12
>> dstapic 33 dstirq 12
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13
>> dstapic 33 dstirq 13
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14
>> dstapic 33 dstirq 14
>> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15
>> dstapic 33 dstirq 15
>>
>> It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs.
>> Other gsi can be considered 1:1 mapping with irq? Or are there other places
>> reflect the mapping between irq and gsi?
>
> It may be uncommon to have overrides for higher GSIs, but I don't think ACPI
> disallows that.
Do you suggest me to add overrides for higher GSIs into array mp_irqs?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 30.05.2024 13:19, Chen, Jiqian wrote:
> On 2024/5/29 20:22, Jan Beulich wrote:
>> On 29.05.2024 13:13, Chen, Jiqian wrote:
>>> On 2024/5/29 15:10, Jan Beulich wrote:
On 29.05.2024 08:56, Chen, Jiqian wrote:
> On 2024/5/29 14:31, Jan Beulich wrote:
>> On 29.05.2024 04:41, Chen, Jiqian wrote:
>>> But I found in function init_irq_data:
>>> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>>> {
>>> int rc;
>>>
>>> desc = irq_to_desc(irq);
>>> desc->irq = irq;
>>>
>>> rc = init_one_irq_desc(desc);
>>> if ( rc )
>>> return rc;
>>> }
>>> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
>>> mapping?
>>
>> No, as explained before. I also don't see how you would derive that from
>> the code above.
> Because here set desc->irq = irq, and it seems there is no other place to
> change this desc->irq, so, gsi 1 is considered to irq 1.
What are you taking this from? The loop bound isn't nr_gsis, and the
iteration
variable isn't in GSI space either; it's in IRQ numbering space. In this
loop
we're merely leveraging that every GSI has a corresponding IRQ;
there are no assumptions made about the mapping between the two. Afaics at
least.
>> "nr_irqs_gsi" describes what its name says: The number of
>> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI
>> (i.e.
>> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
>> mapping.
>>
>>> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
>>> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
>>> irq_desc directly.
>>
>> Which may be wrong, while that wrong-ness may not have hit anyone in
>> practice (for reasons that would need working out).
>>
>>> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>>
>> Again - no.
> Since you are certain that they are not equal, could you tell me where
> show they are not equal or where build their mappings,
> so that I can know how to do a conversion gsi from irq.
I did point you at the ACPI Interrupt Source Override structure before.
We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
start going from.
>>> Oh! I think I know.
>>> If I want to transform gsi to irq, I need to do below:
>>> int irq, entry, ioapic, pin;
>>>
>>> ioapic = mp_find_ioapic(gsi);
>>> pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
>>> entry = find_irq_entry(ioapic, pin, mp_INT);
>>> irq = pin_2_irq(entry, ioapic, pin);
>>>
>>> Am I right?
>>
>> This looks plausible, yes.
> I dump all mpc_config_intsrc of array mp_irqs, it shows:
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0 dstapic
> 33 dstirq 2
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9 dstapic
> 33 dstirq 9
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1 dstapic
> 33 dstirq 1
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3 dstapic
> 33 dstirq 3
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4 dstapic
> 33 dstirq 4
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5 dstapic
> 33 dstirq 5
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6 dstapic
> 33 dstirq 6
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7 dstapic
> 33 dstirq 7
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8 dstapic
> 33 dstirq 8
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10 dstapic
> 33 dstirq 10
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11 dstapic
> 33 dstirq 11
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12 dstapic
> 33 dstirq 12
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13 dstapic
> 33 dstirq 13
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14 dstapic
> 33 dstirq 14
> (XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15 dstapic
> 33 dstirq 15
>
> It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs.
> Other gsi can be considered 1:1 mapping with irq? Or are there other places
> reflect the mapping between irq and gsi?
It may be uncommon to have overrides for higher GSIs, but I don't think ACPI
disallows that.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/29 20:22, Jan Beulich wrote:
> On 29.05.2024 13:13, Chen, Jiqian wrote:
>> On 2024/5/29 15:10, Jan Beulich wrote:
>>> On 29.05.2024 08:56, Chen, Jiqian wrote:
On 2024/5/29 14:31, Jan Beulich wrote:
> On 29.05.2024 04:41, Chen, Jiqian wrote:
>> But I found in function init_irq_data:
>> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>> {
>> int rc;
>>
>> desc = irq_to_desc(irq);
>> desc->irq = irq;
>>
>> rc = init_one_irq_desc(desc);
>> if ( rc )
>> return rc;
>> }
>> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
>> mapping?
>
> No, as explained before. I also don't see how you would derive that from
> the code above.
Because here set desc->irq = irq, and it seems there is no other place to
change this desc->irq, so, gsi 1 is considered to irq 1.
>>>
>>> What are you taking this from? The loop bound isn't nr_gsis, and the
>>> iteration
>>> variable isn't in GSI space either; it's in IRQ numbering space. In this
>>> loop
>>> we're merely leveraging that every GSI has a corresponding IRQ;
>>> there are no assumptions made about the mapping between the two. Afaics at
>>> least.
>>>
> "nr_irqs_gsi" describes what its name says: The number of
> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
> mapping.
>
>> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
>> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
>> irq_desc directly.
>
> Which may be wrong, while that wrong-ness may not have hit anyone in
> practice (for reasons that would need working out).
>
>> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>
> Again - no.
Since you are certain that they are not equal, could you tell me where
show they are not equal or where build their mappings,
so that I can know how to do a conversion gsi from irq.
>>>
>>> I did point you at the ACPI Interrupt Source Override structure before.
>>> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
>>> start going from.
>> Oh! I think I know.
>> If I want to transform gsi to irq, I need to do below:
>> int irq, entry, ioapic, pin;
>>
>> ioapic = mp_find_ioapic(gsi);
>> pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
>> entry = find_irq_entry(ioapic, pin, mp_INT);
>> irq = pin_2_irq(entry, ioapic, pin);
>>
>> Am I right?
>
> This looks plausible, yes.
I dump all mpc_config_intsrc of array mp_irqs, it shows:
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 0 dstapic 33
dstirq 2
(XEN) find_irq_entry type 3 irqtype 0 irqflag 15 srcbus 0 srcbusirq 9 dstapic
33 dstirq 9
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 1 dstapic 33
dstirq 1
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 3 dstapic 33
dstirq 3
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 4 dstapic 33
dstirq 4
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 5 dstapic 33
dstirq 5
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 6 dstapic 33
dstirq 6
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 7 dstapic 33
dstirq 7
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 8 dstapic 33
dstirq 8
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 10 dstapic
33 dstirq 10
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 11 dstapic
33 dstirq 11
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 12 dstapic
33 dstirq 12
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 13 dstapic
33 dstirq 13
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 14 dstapic
33 dstirq 14
(XEN) find_irq_entry type 3 irqtype 0 irqflag 0 srcbus 0 srcbusirq 15 dstapic
33 dstirq 15
It seems only Legacy irq and gsi[0:15] has a mapping in mp_irqs.
Other gsi can be considered 1:1 mapping with irq? Or are there other places
reflect the mapping between irq and gsi?
And my code will be:
case XEN_DOMCTL_gsi_permission:
{
unsigned int gsi = domctl->u.gsi_permission.gsi;
int irq;
bool allow = domctl->u.gsi_permission.allow_access;
/*
* gsi[0,15] is not 1:1 mapping to legacy irq[0,15], it need a
* transformation. Other gsi is considered be 1:1 mapping to irq
*/
if ( gsi < 16 )
irq = gsi_2_irq(gsi);
else
irq = gsi;
/*
* If current domain is PV or it has PIRQ flag, it has a mapping
* of gsi, pirq and irq, so it should use XEN_DOMCTL_irq_permission
* to grant irq permission.
*/
if (
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 29.05.2024 13:13, Chen, Jiqian wrote:
> On 2024/5/29 15:10, Jan Beulich wrote:
>> On 29.05.2024 08:56, Chen, Jiqian wrote:
>>> On 2024/5/29 14:31, Jan Beulich wrote:
On 29.05.2024 04:41, Chen, Jiqian wrote:
> But I found in function init_irq_data:
> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
> {
> int rc;
>
> desc = irq_to_desc(irq);
> desc->irq = irq;
>
> rc = init_one_irq_desc(desc);
> if ( rc )
> return rc;
> }
> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1
> mapping?
No, as explained before. I also don't see how you would derive that from
the code above.
>>> Because here set desc->irq = irq, and it seems there is no other place to
>>> change this desc->irq, so, gsi 1 is considered to irq 1.
>>
>> What are you taking this from? The loop bound isn't nr_gsis, and the
>> iteration
>> variable isn't in GSI space either; it's in IRQ numbering space. In this loop
>> we're merely leveraging that every GSI has a corresponding IRQ;
>> there are no assumptions made about the mapping between the two. Afaics at
>> least.
>>
"nr_irqs_gsi" describes what its name says: The number of
IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
mapping.
> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
> irq_desc directly.
Which may be wrong, while that wrong-ness may not have hit anyone in
practice (for reasons that would need working out).
> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
Again - no.
>>> Since you are certain that they are not equal, could you tell me where show
>>> they are not equal or where build their mappings,
>>> so that I can know how to do a conversion gsi from irq.
>>
>> I did point you at the ACPI Interrupt Source Override structure before.
>> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
>> start going from.
> Oh! I think I know.
> If I want to transform gsi to irq, I need to do below:
> int irq, entry, ioapic, pin;
>
> ioapic = mp_find_ioapic(gsi);
> pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
> entry = find_irq_entry(ioapic, pin, mp_INT);
> irq = pin_2_irq(entry, ioapic, pin);
>
> Am I right?
This looks plausible, yes.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/29 15:10, Jan Beulich wrote:
> On 29.05.2024 08:56, Chen, Jiqian wrote:
>> On 2024/5/29 14:31, Jan Beulich wrote:
>>> On 29.05.2024 04:41, Chen, Jiqian wrote:
Hi,
On 2024/5/17 19:50, Jan Beulich wrote:
> On 17.05.2024 13:14, Chen, Jiqian wrote:
>> On 2024/5/17 18:51, Jan Beulich wrote:
>>> On 17.05.2024 12:45, Chen, Jiqian wrote:
On 2024/5/16 22:01, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> +if ( gsi >= nr_irqs_gsi )
>> +{
>> +ret = -EINVAL;
>> +break;
>> +}
>> +
>> +if ( !irq_access_permitted(current->domain, gsi) ||
>
> I.e. assuming IRQ == GSI? Is that a valid assumption when any number
> of
> source overrides may be surfaced by ACPI?
All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>>>
>>> They are, but there's not necessarily a 1:1 mapping.
>> Oh, so do I need to add a new gsi_caps to store granted gsi?
>
> Probably not. You ought to be able to translate between GSI and IRQ,
> and then continue to record in / check against IRQ permissions.
But I found in function init_irq_data:
for ( irq = 0; irq < nr_irqs_gsi; irq++ )
{
int rc;
desc = irq_to_desc(irq);
desc->irq = irq;
rc = init_one_irq_desc(desc);
if ( rc )
return rc;
}
Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1 mapping?
>>>
>>> No, as explained before. I also don't see how you would derive that from
>>> the code above.
>> Because here set desc->irq = irq, and it seems there is no other place to
>> change this desc->irq, so, gsi 1 is considered to irq 1.
>
> What are you taking this from? The loop bound isn't nr_gsis, and the iteration
> variable isn't in GSI space either; it's in IRQ numbering space. In this loop
> we're merely leveraging that every GSI has a corresponding IRQ;
> there are no assumptions made about the mapping between the two. Afaics at
> least.
>
>>> "nr_irqs_gsi" describes what its name says: The number of
>>> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
>>> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
>>> mapping.
>>>
What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
irq_desc directly.
>>>
>>> Which may be wrong, while that wrong-ness may not have hit anyone in
>>> practice (for reasons that would need working out).
>>>
Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>>>
>>> Again - no.
>> Since you are certain that they are not equal, could you tell me where show
>> they are not equal or where build their mappings,
>> so that I can know how to do a conversion gsi from irq.
>
> I did point you at the ACPI Interrupt Source Override structure before.
> We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
> start going from.
Oh! I think I know.
If I want to transform gsi to irq, I need to do below:
int irq, entry, ioapic, pin;
ioapic = mp_find_ioapic(gsi);
pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
entry = find_irq_entry(ioapic, pin, mp_INT);
irq = pin_2_irq(entry, ioapic, pin);
Am I right?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 29.05.2024 08:56, Chen, Jiqian wrote:
> On 2024/5/29 14:31, Jan Beulich wrote:
>> On 29.05.2024 04:41, Chen, Jiqian wrote:
>>> Hi,
>>> On 2024/5/17 19:50, Jan Beulich wrote:
On 17.05.2024 13:14, Chen, Jiqian wrote:
> On 2024/5/17 18:51, Jan Beulich wrote:
>> On 17.05.2024 12:45, Chen, Jiqian wrote:
>>> On 2024/5/16 22:01, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
> +if ( gsi >= nr_irqs_gsi )
> +{
> +ret = -EINVAL;
> +break;
> +}
> +
> +if ( !irq_access_permitted(current->domain, gsi) ||
I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
source overrides may be surfaced by ACPI?
>>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>>
>> They are, but there's not necessarily a 1:1 mapping.
> Oh, so do I need to add a new gsi_caps to store granted gsi?
Probably not. You ought to be able to translate between GSI and IRQ,
and then continue to record in / check against IRQ permissions.
>>> But I found in function init_irq_data:
>>> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>>> {
>>> int rc;
>>>
>>> desc = irq_to_desc(irq);
>>> desc->irq = irq;
>>>
>>> rc = init_one_irq_desc(desc);
>>> if ( rc )
>>> return rc;
>>> }
>>> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1 mapping?
>>
>> No, as explained before. I also don't see how you would derive that from the
>> code above.
> Because here set desc->irq = irq, and it seems there is no other place to
> change this desc->irq, so, gsi 1 is considered to irq 1.
What are you taking this from? The loop bound isn't nr_gsis, and the iteration
variable isn't in GSI space either; it's in IRQ numbering space. In this loop
we're merely leveraging that every GSI has a corresponding IRQ; there are no
assumptions made about the mapping between the two. Afaics at least.
>> "nr_irqs_gsi" describes what its name says: The number of
>> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
>> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
>> mapping.
>>
>>> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
>>> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get
>>> irq_desc directly.
>>
>> Which may be wrong, while that wrong-ness may not have hit anyone in
>> practice (for reasons that would need working out).
>>
>>> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>>
>> Again - no.
> Since you are certain that they are not equal, could you tell me where show
> they are not equal or where build their mappings,
> so that I can know how to do a conversion gsi from irq.
I did point you at the ACPI Interrupt Source Override structure before.
We're parsing those in acpi_parse_int_src_ovr(), to give you a place to
start going from.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/29 14:31, Jan Beulich wrote:
> On 29.05.2024 04:41, Chen, Jiqian wrote:
>> Hi,
>> On 2024/5/17 19:50, Jan Beulich wrote:
>>> On 17.05.2024 13:14, Chen, Jiqian wrote:
On 2024/5/17 18:51, Jan Beulich wrote:
> On 17.05.2024 12:45, Chen, Jiqian wrote:
>> On 2024/5/16 22:01, Jan Beulich wrote:
>>> On 16.05.2024 11:52, Jiqian Chen wrote:
+if ( gsi >= nr_irqs_gsi )
+{
+ret = -EINVAL;
+break;
+}
+
+if ( !irq_access_permitted(current->domain, gsi) ||
>>>
>>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>>> source overrides may be surfaced by ACPI?
>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>
> They are, but there's not necessarily a 1:1 mapping.
Oh, so do I need to add a new gsi_caps to store granted gsi?
>>>
>>> Probably not. You ought to be able to translate between GSI and IRQ,
>>> and then continue to record in / check against IRQ permissions.
>> But I found in function init_irq_data:
>> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>> {
>> int rc;
>>
>> desc = irq_to_desc(irq);
>> desc->irq = irq;
>>
>> rc = init_one_irq_desc(desc);
>> if ( rc )
>> return rc;
>> }
>> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1 mapping?
>
> No, as explained before. I also don't see how you would derive that from the
> code above.
Because here set desc->irq = irq, and it seems there is no other place to
change this desc->irq, so, gsi 1 is considered to irq 1.
> "nr_irqs_gsi" describes what its name says: The number of
> IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
> mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
> mapping.
>
>> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
>> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get irq_desc
>> directly.
>
> Which may be wrong, while that wrong-ness may not have hit anyone in
> practice (for reasons that would need working out).
>
>> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>
> Again - no.
Since you are certain that they are not equal, could you tell me where show
they are not equal or where build their mappings,
so that I can know how to do a conversion gsi from irq.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 29.05.2024 04:41, Chen, Jiqian wrote:
> Hi,
> On 2024/5/17 19:50, Jan Beulich wrote:
>> On 17.05.2024 13:14, Chen, Jiqian wrote:
>>> On 2024/5/17 18:51, Jan Beulich wrote:
On 17.05.2024 12:45, Chen, Jiqian wrote:
> On 2024/5/16 22:01, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> +if ( gsi >= nr_irqs_gsi )
>>> +{
>>> +ret = -EINVAL;
>>> +break;
>>> +}
>>> +
>>> +if ( !irq_access_permitted(current->domain, gsi) ||
>>
>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>> source overrides may be surfaced by ACPI?
> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
They are, but there's not necessarily a 1:1 mapping.
>>> Oh, so do I need to add a new gsi_caps to store granted gsi?
>>
>> Probably not. You ought to be able to translate between GSI and IRQ,
>> and then continue to record in / check against IRQ permissions.
> But I found in function init_irq_data:
> for ( irq = 0; irq < nr_irqs_gsi; irq++ )
> {
> int rc;
>
> desc = irq_to_desc(irq);
> desc->irq = irq;
>
> rc = init_one_irq_desc(desc);
> if ( rc )
> return rc;
> }
> Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1 mapping?
No, as explained before. I also don't see how you would derive that from
the code above. "nr_irqs_gsi" describes what its name says: The number of
IRQs mapping to a (_some_) GSI. That's to tell them from the non-GSI (i.e.
mainly MSI) ones. There's no implication whatsoever on the IRQ <-> GSI
mapping.
> What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
> and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get irq_desc
> directly.
Which may be wrong, while that wrong-ness may not have hit anyone in
practice (for reasons that would need working out).
> Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
Again - no.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
Hi,
On 2024/5/17 19:50, Jan Beulich wrote:
> On 17.05.2024 13:14, Chen, Jiqian wrote:
>> On 2024/5/17 18:51, Jan Beulich wrote:
>>> On 17.05.2024 12:45, Chen, Jiqian wrote:
On 2024/5/16 22:01, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> +if ( gsi >= nr_irqs_gsi )
>> +{
>> +ret = -EINVAL;
>> +break;
>> +}
>> +
>> +if ( !irq_access_permitted(current->domain, gsi) ||
>
> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
> source overrides may be surfaced by ACPI?
All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>>>
>>> They are, but there's not necessarily a 1:1 mapping.
>> Oh, so do I need to add a new gsi_caps to store granted gsi?
>
> Probably not. You ought to be able to translate between GSI and IRQ,
> and then continue to record in / check against IRQ permissions.
But I found in function init_irq_data:
for ( irq = 0; irq < nr_irqs_gsi; irq++ )
{
int rc;
desc = irq_to_desc(irq);
desc->irq = irq;
rc = init_one_irq_desc(desc);
if ( rc )
return rc;
}
Does it mean that when irq < nr_irqs_gsi, the gsi and irq is a 1:1 mapping?
What's more, when using PHYSDEVOP_setup_gsi, it calls mp_register_gsi,
and in mp_register_gsi, it uses " desc = irq_to_desc(gsi); " to get irq_desc
directly.
Combining above, can we consider "gsi == irq" when irq < nr_irqs_gsi ?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 17.05.2024 13:14, Chen, Jiqian wrote:
> On 2024/5/17 18:51, Jan Beulich wrote:
>> On 17.05.2024 12:45, Chen, Jiqian wrote:
>>> On 2024/5/16 22:01, Jan Beulich wrote:
On 16.05.2024 11:52, Jiqian Chen wrote:
> +if ( gsi >= nr_irqs_gsi )
> +{
> +ret = -EINVAL;
> +break;
> +}
> +
> +if ( !irq_access_permitted(current->domain, gsi) ||
I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
source overrides may be surfaced by ACPI?
>>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>>
>> They are, but there's not necessarily a 1:1 mapping.
> Oh, so do I need to add a new gsi_caps to store granted gsi?
Probably not. You ought to be able to translate between GSI and IRQ,
and then continue to record in / check against IRQ permissions.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/17 18:51, Jan Beulich wrote:
> On 17.05.2024 12:45, Chen, Jiqian wrote:
>> On 2024/5/16 22:01, Jan Beulich wrote:
>>> On 16.05.2024 11:52, Jiqian Chen wrote:
+if ( gsi >= nr_irqs_gsi )
+{
+ret = -EINVAL;
+break;
+}
+
+if ( !irq_access_permitted(current->domain, gsi) ||
>>>
>>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>>> source overrides may be surfaced by ACPI?
>> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>
> They are, but there's not necessarily a 1:1 mapping.
Oh, so do I need to add a new gsi_caps to store granted gsi?
And Dom0 defaults to own all gsis permission?
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 17.05.2024 12:45, Chen, Jiqian wrote:
> On 2024/5/16 22:01, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> +if ( gsi >= nr_irqs_gsi )
>>> +{
>>> +ret = -EINVAL;
>>> +break;
>>> +}
>>> +
>>> +if ( !irq_access_permitted(current->domain, gsi) ||
>>
>> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
>> source overrides may be surfaced by ACPI?
> All irqs smaller than nr_irqs_gsi are gsi, aren't they?
They are, but there's not necessarily a 1:1 mapping.
Jan
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 2024/5/16 22:01, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> Some type of domain don't have PIRQ, like PVH, when
>> passthrough a device to guest on PVH dom0, callstack
>> pci_add_dm_done->XEN_DOMCTL_irq_permission will failed
>> at domain_pirq_to_irq.
>>
>> So, add a new hypercall to grant/revoke gsi permission
>> when dom0 is not PV or dom0 has not PIRQ flag.
>
> Honestly I find this hard to follow, and thus not really making clear why
> no other existing mechanism could be used.
Sorry, I will describe more clearly in next version.
>
>> Signed-off-by: Huang Rui
>> Signed-off-by: Jiqian Chen
>> ---
>
> Below here in an RFC patch you typically would want to put specific items
> you're seeking feedback on. Without that it's hard to tell why this is
> marked RFC.
OK, I will add " RFC: wait for the third patch on kernel side is accepted." in
next version.
>
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -237,6 +237,37 @@ long arch_do_domctl(
>> break;
>> }
>>
>> +case XEN_DOMCTL_gsi_permission:
>> +{
>> +unsigned int gsi = domctl->u.gsi_permission.gsi;
>> +int allow = domctl->u.gsi_permission.allow_access;
>
> bool?
Will change.
>
>> +if ( is_pv_domain(current->domain) || has_pirq(current->domain) )
>> +{
>> +ret = -EOPNOTSUPP;
>> +break;
>> +}
>
> Such a restriction imo wants explaining in a comment.
Will add in next version.
>
>> +if ( gsi >= nr_irqs_gsi )
>> +{
>> +ret = -EINVAL;
>> +break;
>> +}
>> +
>> +if ( !irq_access_permitted(current->domain, gsi) ||
>
> I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
> source overrides may be surfaced by ACPI?
All irqs smaller than nr_irqs_gsi are gsi, aren't they?
>
>> + xsm_irq_permission(XSM_HOOK, d, gsi, allow) )
>
> Here I'm pretty sure you can't very well re-use an existing hook, as the
> value of interest is in a different numbering space, and a possible hook
> function has no way of knowing which one it is. Daniel?
>
>> +{
>> +ret = -EPERM;
>> +break;
>> +}
>> +
>> +if ( allow )
>> +ret = irq_permit_access(d, gsi);
>> +else
>> +ret = irq_deny_access(d, gsi);
>
> As above I'm afraid you can't assume IRQ == GSI.
>
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -447,6 +447,13 @@ struct xen_domctl_irq_permission {
>> };
>>
>>
>> +/* XEN_DOMCTL_gsi_permission */
>> +struct xen_domctl_gsi_permission {
>> +uint32_t gsi;
>> +uint8_t allow_access;/* flag to specify enable/disable of x86 gsi
>> access */
>> +};
>
> Explicit padding please, including a check that it's zero on input.
Thanks, I will add in next version.
>
>> +
>> +
>> /* XEN_DOMCTL_iomem_permission */
>
> No double blank lines please. In fact you will want to break the double blank
> lines in leading context, inserting in the middle.
Will remove one.
>
> Jan
--
Best regards,
Jiqian Chen.
Re: [RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On 16.05.2024 11:52, Jiqian Chen wrote:
> Some type of domain don't have PIRQ, like PVH, when
> passthrough a device to guest on PVH dom0, callstack
> pci_add_dm_done->XEN_DOMCTL_irq_permission will failed
> at domain_pirq_to_irq.
>
> So, add a new hypercall to grant/revoke gsi permission
> when dom0 is not PV or dom0 has not PIRQ flag.
Honestly I find this hard to follow, and thus not really making clear why
no other existing mechanism could be used.
> Signed-off-by: Huang Rui
> Signed-off-by: Jiqian Chen
> ---
Below here in an RFC patch you typically would want to put specific items
you're seeking feedback on. Without that it's hard to tell why this is
marked RFC.
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -237,6 +237,37 @@ long arch_do_domctl(
> break;
> }
>
> +case XEN_DOMCTL_gsi_permission:
> +{
> +unsigned int gsi = domctl->u.gsi_permission.gsi;
> +int allow = domctl->u.gsi_permission.allow_access;
bool?
> +if ( is_pv_domain(current->domain) || has_pirq(current->domain) )
> +{
> +ret = -EOPNOTSUPP;
> +break;
> +}
Such a restriction imo wants explaining in a comment.
> +if ( gsi >= nr_irqs_gsi )
> +{
> +ret = -EINVAL;
> +break;
> +}
> +
> +if ( !irq_access_permitted(current->domain, gsi) ||
I.e. assuming IRQ == GSI? Is that a valid assumption when any number of
source overrides may be surfaced by ACPI?
> + xsm_irq_permission(XSM_HOOK, d, gsi, allow) )
Here I'm pretty sure you can't very well re-use an existing hook, as the
value of interest is in a different numbering space, and a possible hook
function has no way of knowing which one it is. Daniel?
> +{
> +ret = -EPERM;
> +break;
> +}
> +
> +if ( allow )
> +ret = irq_permit_access(d, gsi);
> +else
> +ret = irq_deny_access(d, gsi);
As above I'm afraid you can't assume IRQ == GSI.
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -447,6 +447,13 @@ struct xen_domctl_irq_permission {
> };
>
>
> +/* XEN_DOMCTL_gsi_permission */
> +struct xen_domctl_gsi_permission {
> +uint32_t gsi;
> +uint8_t allow_access;/* flag to specify enable/disable of x86 gsi
> access */
> +};
Explicit padding please, including a check that it's zero on input.
> +
> +
> /* XEN_DOMCTL_iomem_permission */
No double blank lines please. In fact you will want to break the double blank
lines in leading context, inserting in the middle.
Jan
[RFC XEN PATCH v8 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
Some type of domain don't have PIRQ, like PVH, when
passthrough a device to guest on PVH dom0, callstack
pci_add_dm_done->XEN_DOMCTL_irq_permission will failed
at domain_pirq_to_irq.
So, add a new hypercall to grant/revoke gsi permission
when dom0 is not PV or dom0 has not PIRQ flag.
Signed-off-by: Huang Rui
Signed-off-by: Jiqian Chen
---
tools/include/xenctrl.h | 5 +++
tools/libs/ctrl/xc_domain.c | 15
tools/libs/light/libxl_pci.c | 72
xen/arch/x86/domctl.c| 31
xen/include/public/domctl.h | 9 +
xen/xsm/flask/hooks.c| 1 +
6 files changed, 117 insertions(+), 16 deletions(-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 841db41ad7e4..c21a79d74be3 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1382,6 +1382,11 @@ int xc_domain_irq_permission(xc_interface *xch,
uint32_t pirq,
bool allow_access);
+int xc_domain_gsi_permission(xc_interface *xch,
+ uint32_t domid,
+ uint32_t gsi,
+ bool allow_access);
+
int xc_domain_iomem_permission(xc_interface *xch,
uint32_t domid,
unsigned long first_mfn,
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index f2d9d14b4d9f..8540e84fda93 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -1394,6 +1394,21 @@ int xc_domain_irq_permission(xc_interface *xch,
return do_domctl(xch, );
}
+int xc_domain_gsi_permission(xc_interface *xch,
+ uint32_t domid,
+ uint32_t gsi,
+ bool allow_access)
+{
+struct xen_domctl domctl = {
+.cmd = XEN_DOMCTL_gsi_permission,
+.domain = domid,
+.u.gsi_permission.gsi = gsi,
+.u.gsi_permission.allow_access = allow_access,
+};
+
+return do_domctl(xch, );
+}
+
int xc_domain_iomem_permission(xc_interface *xch,
uint32_t domid,
unsigned long first_mfn,
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 7e44d4c3ae2b..1d1b81dd2844 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -1412,6 +1412,37 @@ static bool pci_supp_legacy_irq(void)
#define PCI_SBDF(seg, bus, devfn) \
uint32_t)(seg)) << 16) | (PCI_DEVID(bus, devfn)))
+static int pci_device_set_gsi(libxl_ctx *ctx,
+ libxl_domid domid,
+ libxl_device_pci *pci,
+ bool map,
+ int *gsi_back)
+{
+int r, gsi, pirq;
+uint32_t sbdf;
+
+sbdf = PCI_SBDF(pci->domain, pci->bus, (PCI_DEVFN(pci->dev, pci->func)));
+r = xc_physdev_gsi_from_dev(ctx->xch, sbdf);
+*gsi_back = r;
+if (r < 0)
+return r;
+
+gsi = r;
+pirq = r;
+if (map)
+r = xc_physdev_map_pirq(ctx->xch, domid, gsi, );
+else
+r = xc_physdev_unmap_pirq(ctx->xch, domid, pirq);
+if (r)
+return r;
+
+r = xc_domain_gsi_permission(ctx->xch, domid, gsi, map);
+if (r && errno == EOPNOTSUPP)
+r = xc_domain_irq_permission(ctx->xch, domid, gsi, map);
+
+return r;
+}
+
static void pci_add_dm_done(libxl__egc *egc,
pci_add_state *pas,
int rc)
@@ -1424,10 +1455,10 @@ static void pci_add_dm_done(libxl__egc *egc,
unsigned long long start, end, flags, size;
int irq, i;
int r;
-uint32_t sbdf;
uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
uint32_t domainid = domid;
bool isstubdom = libxl_is_stubdom(ctx, domid, );
+int gsi;
/* Convenience aliases */
bool starting = pas->starting;
@@ -1485,6 +1516,19 @@ static void pci_add_dm_done(libxl__egc *egc,
fclose(f);
if (!pci_supp_legacy_irq())
goto out_no_irq;
+
+r = pci_device_set_gsi(ctx, domid, pci, 1, );
+if (gsi >= 0) {
+if (r < 0) {
+rc = ERROR_FAIL;
+LOGED(ERROR, domainid,
+ "pci_device_set_gsi gsi=%d (error=%d)", gsi, errno);
+goto out;
+} else {
+goto process_permissive;
+}
+}
+/* if gsi < 0, keep using irq */
sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain,
pci->bus, pci->dev, pci->func);
f = fopen(sysfs_path, "r");
@@ -1493,13 +1537,6 @@ static void pci_add_dm_done(libxl__egc *egc,
goto out_no_irq;
}
if ((fscanf(f, "%u", ) == 1) && irq) {
-sbdf = PCI_SBDF(pci->domain, pci->bus,
-(PCI_DEVFN(pci->dev, pci->func)));
-r = xc_physdev_gsi_from_dev(ctx->xch, sbdf);
-/* if fail, keep using irq; if
