| 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: <0d249e3e-6189-3dc4-2d9e-d1c9291ddab1@redhat.com>
Date: Thu, 16 Mar 2017 09:59:20 +0100
From: Auger Eric <eric.auger@...hat.com>
To: Marc Zyngier <marc.zyngier@....com>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu
Cc: Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>,
Christoffer Dall <christoffer.dall@...aro.org>
Subject: Re: [RFC PATCH 18/33] irqchip/gic-v3-its: Add VLPI map/unmap
operations
Hi,
On 17/01/2017 11:20, Marc Zyngier wrote:
> In order to let a VLPI being injected into a guest, the VLPI must
> be mapped using the VMAPTI command. When moved to a different vcpu,
> it must be moved with the VMOVI command.
>
> These commands are issued via the irq_set_vcpu_affinity method,
> making sure we unmap the corresponding host LPI first.
>
> The reverse is also done when the VLPI is unmapped from the guest.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 174 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 174 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 1bd78ca..d50a019 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -115,6 +115,7 @@ struct its_node {
> struct event_lpi_map {
> unsigned long *lpi_map;
> u16 *col_map;
> + struct its_vlpi_map *vlpi_map;
> irq_hw_number_t lpi_base;
> int nr_lpis;
> };
> @@ -201,6 +202,21 @@ struct its_cmd_desc {
> struct {
> struct its_collection *col;
> } its_invall_cmd;
> +
> + struct {
> + struct its_vpe *vpe;
> + struct its_device *dev;
> + u32 virt_id;
> + u32 event_id;
> + bool db_enabled;
> + } its_vmapti_cmd;
> +
> + struct {
> + struct its_vpe *vpe;
> + struct its_device *dev;
> + u32 event_id;
> + bool db_enabled;
> + } its_vmovi_cmd;
> };
> };
>
> @@ -217,6 +233,9 @@ struct its_cmd_block {
> typedef struct its_collection *(*its_cmd_builder_t)(struct its_cmd_block *,
> struct its_cmd_desc *);
>
> +typedef struct its_vpe *(*its_cmd_vbuilder_t)(struct its_cmd_block *,
> + struct its_cmd_desc *);
> +
> static void its_mask_encode(u64 *raw_cmd, u64 val, int h, int l)
> {
> u64 mask = GENMASK_ULL(h, l);
> @@ -269,6 +288,26 @@ static void its_encode_collection(struct its_cmd_block *cmd, u16 col)
> its_mask_encode(&cmd->raw_cmd[2], col, 15, 0);
> }
>
> +static void its_encode_vpeid(struct its_cmd_block *cmd, u16 vpeid)
> +{
> + its_mask_encode(&cmd->raw_cmd[1], vpeid, 47, 32);
> +}
> +
> +static void its_encode_virt_id(struct its_cmd_block *cmd, u32 virt_id)
> +{
> + its_mask_encode(&cmd->raw_cmd[2], virt_id, 31, 0);
> +}
> +
> +static void its_encode_db_phys_id(struct its_cmd_block *cmd, u32 db_phys_id)
> +{
> + its_mask_encode(&cmd->raw_cmd[2], db_phys_id, 63, 32);
> +}
> +
> +static void its_encode_db_valid(struct its_cmd_block *cmd, bool db_valid)
> +{
> + its_mask_encode(&cmd->raw_cmd[2], db_valid, 0, 0);
> +}
> +
> static inline void its_fixup_cmd(struct its_cmd_block *cmd)
> {
> /* Let's fixup BE commands */
> @@ -427,6 +466,50 @@ static struct its_collection *its_build_invall_cmd(struct its_cmd_block *cmd,
> return NULL;
> }
>
> +static struct its_vpe *its_build_vmapti_cmd(struct its_cmd_block *cmd,
> + struct its_cmd_desc *desc)
> +{
> + u32 db;
> +
> + if (desc->its_vmapti_cmd.db_enabled)
> + db = desc->its_vmapti_cmd.vpe->vpe_db_lpi;
> + else
> + db = 1023;
> +
> + its_encode_cmd(cmd, GITS_CMD_VMAPTI);
> + its_encode_devid(cmd, desc->its_vmapti_cmd.dev->device_id);
> + its_encode_vpeid(cmd, desc->its_vmapti_cmd.vpe->vpe_id);
> + its_encode_event_id(cmd, desc->its_vmapti_cmd.event_id);
> + its_encode_db_phys_id(cmd, db);
> + its_encode_virt_id(cmd, desc->its_vmapti_cmd.virt_id);
> +
> + its_fixup_cmd(cmd);
> +
> + return desc->its_vmapti_cmd.vpe;
> +}
> +
> +static struct its_vpe *its_build_vmovi_cmd(struct its_cmd_block *cmd,
> + struct its_cmd_desc *desc)
> +{
> + u32 db;
> +
> + if (desc->its_vmovi_cmd.db_enabled)
> + db = desc->its_vmovi_cmd.vpe->vpe_db_lpi;
> + else
> + db = 1023;
> +
> + its_encode_cmd(cmd, GITS_CMD_VMOVI);
> + its_encode_devid(cmd, desc->its_vmovi_cmd.dev->device_id);
> + its_encode_vpeid(cmd, desc->its_vmovi_cmd.vpe->vpe_id);
> + its_encode_event_id(cmd, desc->its_vmovi_cmd.event_id);
> + its_encode_db_phys_id(cmd, db);
> + its_encode_db_valid(cmd, true);
> +
> + its_fixup_cmd(cmd);
> +
> + return desc->its_vmovi_cmd.vpe;
> +}
> +
> static u64 its_cmd_ptr_to_offset(struct its_node *its,
> struct its_cmd_block *ptr)
> {
> @@ -570,6 +653,16 @@ static void its_build_sync_cmd(struct its_cmd_block *sync_cmd,
> static __its_send_single_cmd(its_send_single_command, its_cmd_builder_t,
> struct its_collection, its_build_sync_cmd)
>
> +static void its_build_vsync_cmd(struct its_cmd_block *sync_cmd,
> + struct its_vpe *sync_vpe)
> +{
> + its_encode_cmd(sync_cmd, GITS_CMD_VSYNC);
> + its_encode_vpeid(sync_cmd, sync_vpe->vpe_id);
why don't we need the its_fixup_cmd() here as well? I see it is not
there in its_build_sync_cmd either.
> +}
> +
> +static __its_send_single_cmd(its_send_single_vcommand, its_cmd_vbuilder_t,
> + struct its_vpe, its_build_vsync_cmd)
> +
> static void its_send_int(struct its_device *dev, u32 event_id)
> {
> struct its_cmd_desc desc;
> @@ -663,6 +756,33 @@ static void its_send_invall(struct its_node *its, struct its_collection *col)
> its_send_single_command(its, its_build_invall_cmd, &desc);
> }
>
> +static void its_send_vmapti(struct its_device *dev, u32 id)
> +{
> + struct its_vlpi *vlpi = &dev->event_map.vlpi_map->vlpis[id];
> + struct its_cmd_desc desc;
> +
> + desc.its_vmapti_cmd.vpe = dev->event_map.vlpi_map->vpes[vlpi->vpe_idx];
> + desc.its_vmapti_cmd.dev = dev;
> + desc.its_vmapti_cmd.virt_id = vlpi->vintid;
> + desc.its_vmapti_cmd.event_id = id;
> + desc.its_vmapti_cmd.db_enabled = vlpi->db_enabled;
> +
> + its_send_single_vcommand(dev->its, its_build_vmapti_cmd, &desc);
> +}
> +
> +static void its_send_vmovi(struct its_device *dev, u32 id)
> +{
> + struct its_vlpi *vlpi = &dev->event_map.vlpi_map->vlpis[id];
> + struct its_cmd_desc desc;
> +
> + desc.its_vmovi_cmd.vpe = dev->event_map.vlpi_map->vpes[vlpi->vpe_idx];
> + desc.its_vmovi_cmd.dev = dev;
> + desc.its_vmovi_cmd.event_id = id;
> + desc.its_vmovi_cmd.db_enabled = vlpi->db_enabled;
> +
> + its_send_single_vcommand(dev->its, its_build_vmovi_cmd, &desc);
> +}
> +
> /*
> * irqchip functions - assumes MSI, mostly.
> */
> @@ -754,6 +874,20 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
> iommu_dma_map_msi_msg(d->irq, msg);
> }
>
> +static int its_install_vlpi_map(struct its_device *its_dev,
> + struct its_vlpi_map *map)
> +{
> + struct its_vlpi_map *old_map;
> +
> + if (map->nr_vlpis != its_dev->event_map.nr_lpis)
> + return -EINVAL;
> + old_map = cmpxchg(&its_dev->event_map.vlpi_map, NULL, map);
> + if (old_map && WARN_ON(old_map != map))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int its_irq_set_irqchip_state(struct irq_data *d,
> enum irqchip_irq_state which,
> bool state)
> @@ -785,11 +919,51 @@ static int its_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
> switch (info->cmd_type) {
> case MAP_VLPI:
> {
> + int ret;
> +
> + if (!info->map)
> + return -EINVAL;
> +
> + ret = its_install_vlpi_map(its_dev, info->map);
> + if (ret)
> + return ret;
> +
> + if (irqd_is_forwarded_to_vcpu(d)) {
> + /* Already mapped, move it around */
> + its_send_vmovi(its_dev, event);
> + } else {
> + /* Drop the physical mapping */
> + its_send_discard(its_dev, event);
> +
> + /* and install the virtual one */
> + its_send_vmapti(its_dev, event);
Just a question about error handling. Do we have any guarantee the ITS
commands won't fail?
> + irqd_set_forwarded_to_vcpu(d);
Don't we need a lock to guarantee the forwarded state and the ITS state
are consistent?
don't you need some sanity check on the vINTID?
Thanks
Eric
> +
> + /*
> + * The host will never see that interrupt
> + * firing again, so it is vital that we don't
> + * do any lazy masking.
> + */
> + irq_set_status_flags(d->irq, IRQ_DISABLE_UNLAZY);
> + }
> +
> return 0;
> }
>
> case UNMAP_VLPI:
> {
> + if (!its_dev->event_map.vlpi_map ||
> + !irqd_is_forwarded_to_vcpu(d))
> + return -EINVAL;
> +
> + /* Drop the virtual mapping */
> + its_send_discard(its_dev, event);
> +
> + /* and restore the physical one */
> + irqd_clr_forwarded_to_vcpu(d);
> + irq_clear_status_flags(d->irq, IRQ_DISABLE_UNLAZY);
> + its_send_mapvi(its_dev, d->hwirq, event);
> +
> return 0;
> }
>
>
Powered by blists - more mailing lists