| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DM5PR2101MB0918E63F306E9226A801BF55D76C0@DM5PR2101MB0918.namprd21.prod.outlook.com>
Date: Sun, 3 Feb 2019 21:50:05 +0000
From: Michael Kelley <mikelley@...rosoft.com>
To: "lantianyu1986@...il.com" <lantianyu1986@...il.com>
CC: Tianyu Lan <Tianyu.Lan@...rosoft.com>,
"joro@...tes.org" <joro@...tes.org>,
"mchehab+samsung@...nel.org" <mchehab+samsung@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
"arnd@...db.de" <arnd@...db.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
KY Srinivasan <kys@...rosoft.com>,
vkuznets <vkuznets@...hat.com>,
"alex.williamson@...hat.com" <alex.williamson@...hat.com>,
"sashal@...nel.org" <sashal@...nel.org>,
"dan.carpenter@...cle.com" <dan.carpenter@...cle.com>
Subject: RE: [PATCH V2 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
From: lantianyu1986@...il.com <lantianyu1986@...il.com> Sent: Saturday, February 2, 2019 5:15 AM
>
> +/*
> + * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
> + * Redirection Table.
> + */
> +#define IOAPIC_REMAPPING_ENTRY 24
The other unstated assumption here is that Hyper-v guest VMs
have only a single IOAPIC, regardless of the size of the VM.
Perhaps that should be stated in the comment explaining why
there are 24 entries?
> +
> +static cpumask_t ioapic_max_cpumask = { CPU_BITS_NONE };
> +static struct irq_domain *ioapic_ir_domain;
> +
> +static int hyperv_ir_set_affinity(struct irq_data *data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent = data->parent_data;
> + struct irq_cfg *cfg = irqd_cfg(data);
> + struct IO_APIC_route_entry *entry;
> + cpumask_t cpumask;
> + int ret;
> +
> + cpumask_andnot(&cpumask, mask, &ioapic_max_cpumask);
> +
> + /* Return error If new irq affinity is out of ioapic_max_cpumask. */
> + if (!cpumask_empty(&cpumask))
> + return -EINVAL;
The above two cpumask functions can be combined in a single
call to cpumask_subset(). This has the nice property that determining
whether the cpus in "mask" are a subset of the cpus in "ioapic_max_cpumask"
is exactly what this code is trying to do. :-) And it gets rid of the local
cpumask variable and the associated compiler warnings about stack frame
size.
> +
> + ret = parent->chip->irq_set_affinity(parent, mask, force);
> + if (ret < 0 || ret == IRQ_SET_MASK_OK_DONE)
> + return ret;
> +
> + entry = data->chip_data;
> + entry->dest = cfg->dest_apicid;
> + entry->vector = cfg->vector;
> + send_cleanup_vector(cfg);
> +
> + return 0;
> +}
> +
> +static struct irq_chip hyperv_ir_chip = {
> + .name = "HYPERV-IR",
> + .irq_ack = apic_ack_irq,
> + .irq_set_affinity = hyperv_ir_set_affinity,
> +};
> +
> +static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs,
> + void *arg)
> +{
> + struct irq_alloc_info *info = arg;
> + struct irq_data *irq_data;
> + struct irq_desc *desc;
> + int ret = 0;
> +
> + if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> + return -EINVAL;
> +
> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> + if (ret < 0)
> + return ret;
> +
> + irq_data = irq_domain_get_irq_data(domain, virq);
> + if (!irq_data) {
> + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> + return -EINVAL;
> + }
> +
> + irq_data->chip = &hyperv_ir_chip;
> +
> + /*
> + * IOAPIC entry pointer is saved in chip_data to allow
> + * hyperv_irq_remappng_activate()/hyperv_ir_set_affinity() to set
> + * vector and dest_apicid. cfg->vector and cfg->dest_apicid are
> + * ignorred when IRQ remapping is enabled. See ioapic_configure_entry().
Spelling: "ignored".
I saw Vitaly previous comments, and I still don't understand this comment. :-(
Is IRQ remapping considered to be enabled by this IOMMU driver, such that
cfg->vector and cfg->dest_apicid are ignored? Or is the "when IRQ remapping
is enabled" a statement about some future enhancement?
> + */
> + irq_data->chip_data = info->ioapic_entry;
> +
> + /*
> + * Hypver-V IO APIC irq affinity should be in the scope of
> + * ioapic_max_cpumask because no irq remapping support.
> + */
> + desc = irq_data_to_desc(irq_data);
> + cpumask_and(desc->irq_common_data.affinity,
> + desc->irq_common_data.affinity,
> + &ioapic_max_cpumask);
> +
> + return 0;
> +}
> +
> +static void hyperv_irq_remapping_free(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs)
> +{
> + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +}
> +
> +static int hyperv_irq_remappng_activate(struct irq_domain *domain,
> + struct irq_data *irq_data, bool reserve)
> +{
> + struct irq_cfg *cfg = irqd_cfg(irq_data);
> + struct IO_APIC_route_entry *entry = irq_data->chip_data;
> +
> + entry->dest = cfg->dest_apicid;
> + entry->vector = cfg->vector;
> +
> + return 0;
> +}
> +
> +static struct irq_domain_ops hyperv_ir_domain_ops = {
> + .alloc = hyperv_irq_remapping_alloc,
> + .free = hyperv_irq_remapping_free,
> + .activate = hyperv_irq_remappng_activate,
> +};
> +
> +static int __init hyperv_prepare_irq_remapping(void)
> +{
> + struct fwnode_handle *fn;
> + u32 apic_id;
> + int i;
> +
> + if (x86_hyper_type != X86_HYPER_MS_HYPERV ||
The function hypervisor_is_type() exists for doing the above test.
See include/asm/hypervisor.h
> + !x2apic_supported())
> + return -ENODEV;
> +
> + fn = irq_domain_alloc_named_id_fwnode("HYPERV-IR", 0);
> + if (!fn)
> + return -ENOMEM;
> +
> + ioapic_ir_domain =
> + irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
> + 0, IOAPIC_REMAPPING_ENTRY, fn,
> + &hyperv_ir_domain_ops, NULL);
> +
> + irq_domain_free_fwnode(fn);
> +
> + /*
> + * Hyper-V doesn't provide irq remapping function for
> + * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
> + * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
> + * in the MADT table on Hyper-v are sorted monotonic increasingly.
> + * APIC ID reflects cpu topology. There maybe some APIC ID
> + * gaps when cpu number in a socket is not power of two. Prepare
> + * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
> + * into ioapic_max_cpumask if its APIC ID is less than 256.
> + */
> + for (i = 0; i < 256; i++) {
> + apic_id = cpu_physical_id(i);
> + if (apic_id > 255)
> + continue;
> +
> + cpumask_set_cpu(i, &ioapic_max_cpumask);
> + }
The above for loop would be more compact as:
for (i=0; i <256; i++)
if (cpu_physical_id(i) < 256)
cpumask_set_cpu(i, &ioapic_max_cpumask);
> +
> + return 0;
> +}
> +
Powered by blists - more mailing lists