The GIC driver uses a RMW sequence to update the affinity, and
relies on the gic_lock_irqsave/gic_unlock_irqrestore sequences
to update it atomically.
But these sequences only expend into anything meaningful if
the BL_SWITCHER option is selected, which almost never happens.
It also turns out that using a RMW and locks is just as silly,
as the GIC distributor supports byte accesses for the GICD_TARGETRn
registers, which when used make the update atomic by definition.
Drop the terminally broken code and replace it by a byte write.
Fixes: 04c8b0f82c7d ("irqchip/gic: Make locking a BL_SWITCHER only feature")
Cc: stable(a)vger.kernel.org
Signed-off-by: Marc Zyngier <maz(a)kernel.org>
---
drivers/irqchip/irq-gic.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 00de05abd3c3..c17fabd6741e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -329,10 +329,8 @@ static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
bool force)
{
- void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
- unsigned int cpu, shift = (gic_irq(d) % 4) * 8;
- u32 val, mask, bit;
- unsigned long flags;
+ void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + gic_irq(d);
+ unsigned int cpu;
if (!force)
cpu = cpumask_any_and(mask_val, cpu_online_mask);
@@ -342,13 +340,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
return -EINVAL;
- gic_lock_irqsave(flags);
- mask = 0xff << shift;
- bit = gic_cpu_map[cpu] << shift;
- val = readl_relaxed(reg) & ~mask;
- writel_relaxed(val | bit, reg);
- gic_unlock_irqrestore(flags);
-
+ writeb_relaxed(gic_cpu_map[cpu], reg);
irq_data_update_effective_affinity(d, cpumask_of(cpu));
return IRQ_SET_MASK_OK_DONE;
--
2.27.0
Adds mutex guard to the VMSA updating code. Also adds a check to skip a
vCPU if it has already been LAUNCH_UPDATE_VMSA'd which should allow
userspace to retry this ioctl until all the vCPUs can be successfully
LAUNCH_UPDATE_VMSA'd. Because this operation cannot be undone we cannot
unwind if one vCPU fails.
Fixes: ad73109ae7ec ("KVM: SVM: Provide support to launch and run an SEV-ES guest")
Signed-off-by: Peter Gonda <pgonda(a)google.com>
Cc: Marc Orr <marcorr(a)google.com>
Cc: Paolo Bonzini <pbonzini(a)redhat.com>
Cc: Sean Christopherson <seanjc(a)google.com>
Cc: Brijesh Singh <brijesh.singh(a)amd.com>
Cc: kvm(a)vger.kernel.org
Cc: stable(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
---
arch/x86/kvm/svm/sev.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 75e0b21ad07c..9a2ebd0328ca 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -598,22 +598,29 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
- struct sev_data_launch_update_vmsa vmsa;
+ struct sev_data_launch_update_vmsa vmsa = {0};
struct kvm_vcpu *vcpu;
int i, ret;
if (!sev_es_guest(kvm))
return -ENOTTY;
- vmsa.reserved = 0;
-
kvm_for_each_vcpu(i, vcpu, kvm) {
struct vcpu_svm *svm = to_svm(vcpu);
+ ret = mutex_lock_killable(&vcpu->mutex);
+ if (ret)
+ goto out_unlock;
+
+ /* Skip to the next vCPU if this one has already be updated. */
+ ret = sev_es_sync_vmsa(svm);
+ if (svm->vcpu.arch.guest_state_protected)
+ goto unlock;
+
/* Perform some pre-encryption checks against the VMSA */
ret = sev_es_sync_vmsa(svm);
if (ret)
- return ret;
+ goto out_unlock;
/*
* The LAUNCH_UPDATE_VMSA command will perform in-place
@@ -629,12 +636,19 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_VMSA, &vmsa,
&argp->error);
if (ret)
- return ret;
+ goto out_unlock;
svm->vcpu.arch.guest_state_protected = true;
+
+unlock:
+ mutex_unlock(&vcpu->mutex);
}
return 0;
+
+out_unlock:
+ mutex_unlock(&vcpu->mutex);
+ return ret;
}
static int sev_launch_measure(struct kvm *kvm, struct kvm_sev_cmd *argp)
--
2.33.0.464.g1972c5931b-goog
Good day,
My name is Luis Fernandez.I would like to discuss something
important that will benefit both of us. I will send you more
details upon your response.
Regards
Luis Fernandez
Copying an ASID into new vCPUs will not work for SEV-ES since the vCPUs
VMSAs need to be setup and measured before SEV_LAUNCH_FINISH. Return an
error if a users tries to KVM_CAP_VM_COPY_ENC_CONTEXT_FROM from an
SEV-ES guest. The destination VM is already checked for SEV and SEV-ES
with sev_guest(), so this ioctl already fails if the destination is SEV
enabled.
Enabling mirroring a VM or copying its encryption context with an SEV-ES
VM is more involved and should happen in its own feature patch if that's
needed. This is because the vCPUs of SEV-ES VMs need to be updated with
LAUNCH_UPDATE_VMSA before LAUNCH_FINISH. This needs KVM changes because
the mirror VM has all its SEV ioctls blocked and the original VM doesn't
know about the mirrors vCPUs.
Fixes: 54526d1fd593 ("KVM: x86: Support KVM VMs sharing SEV context")
V2:
* Updated changelog with more information and added stable CC.
Signed-off-by: Peter Gonda <pgonda(a)google.com>
Cc: Marc Orr <marcorr(a)google.com>
Cc: Paolo Bonzini <pbonzini(a)redhat.com>
Cc: Sean Christopherson <seanjc(a)google.com>
Cc: Nathan Tempelman <natet(a)google.com>
Cc: Brijesh Singh <brijesh.singh(a)amd.com>
Cc: kvm(a)vger.kernel.org
Cc: stable(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
---
arch/x86/kvm/svm/sev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 75e0b21ad07c..8a279027425f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1728,7 +1728,7 @@ int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd)
source_kvm = source_kvm_file->private_data;
mutex_lock(&source_kvm->lock);
- if (!sev_guest(source_kvm)) {
+ if (!sev_guest(source_kvm) || sev_es_guest(source_kvm)) {
ret = -EINVAL;
goto e_source_unlock;
}
--
2.33.0.309.g3052b89438-goog
On Tue, Sep 14, 2021 at 11:32 AM Sean Christopherson <seanjc(a)google.com> wrote:
>
> On Tue, Sep 14, 2021, Peter Gonda wrote:
> > Copying an ASID into new vCPUs will not work for SEV-ES since the vCPUs
> > VMSAs need to be setup and measured before SEV_LAUNCH_FINISH. Return an
> > error if a users tries to KVM_CAP_VM_COPY_ENC_CONTEXT_FROM from an
> > SEV-ES guest.
>
> What happens if userspace does KVM_CAP_VM_COPY_ENC_CONTEXT_FROM before the source
> has created vCPUs, i.e. before it has done SEV_LAUNCH_FINISH?
That's not enough. If you wanted to be able to mirror SEV-ES you'd
also need to call LAUNCH_UPDATE_VMSA on the mirror's vCPUs before
SEV_LAUNCH_FINISH. That is do-able but I was writing a small change to
fix this bug. If mirroring of SEV-ES is wanted it's a much bigger
change.
>
> Might be worth noting that the destination cannot be an SEV guest, and therefore
> can't be an SEV-ES guest either.
sev_guest() implies sev_es_guest() so I think this case is covered.
>
> > Fixes: 54526d1fd593 ("KVM: x86: Support KVM VMs sharing SEV context")
>
> Cc: stable(a)vger.kernel.org
Oops. I'll update in the V2 if needed. Added to this thread for now.
If the IR Toy is receiving IR while a transmit is done, it may end up
hanging. We can prevent this from happening by re-entering sample mode
just before issuing the transmit command.
Link: https://github.com/bengtmartensson/HarcHardware/discussions/25
Cc: stable(a)vger.kernel.org
Signed-off-by: Sean Young <sean(a)mess.org>
---
drivers/media/rc/ir_toy.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir_toy.c b/drivers/media/rc/ir_toy.c
index d2d9346eb8f5..71aced52248f 100644
--- a/drivers/media/rc/ir_toy.c
+++ b/drivers/media/rc/ir_toy.c
@@ -26,6 +26,7 @@ static const u8 COMMAND_VERSION[] = { 'v' };
// End transmit and repeat reset command so we exit sump mode
static const u8 COMMAND_RESET[] = { 0xff, 0xff, 0, 0, 0, 0, 0 };
static const u8 COMMAND_SMODE_ENTER[] = { 's' };
+static const u8 COMMAND_SMODE_EXIT[] = { 0 };
static const u8 COMMAND_TXSTART[] = { 0x26, 0x24, 0x25, 0x03 };
#define REPLY_XMITCOUNT 't'
@@ -317,12 +318,30 @@ static int irtoy_tx(struct rc_dev *rc, uint *txbuf, uint count)
buf[i] = cpu_to_be16(v);
}
- buf[count] = cpu_to_be16(0xffff);
+ buf[count] = 0xffff;
irtoy->tx_buf = buf;
irtoy->tx_len = size;
irtoy->emitted = 0;
+ // There is an issue where if the unit is receiving IR while the
+ // first TXSTART command is sent, the device might end up hanging
+ // with its led on. It does not respond to any command when this
+ // happens. To work around this, re-enter sample mode.
+ err = irtoy_command(irtoy, COMMAND_SMODE_EXIT,
+ sizeof(COMMAND_SMODE_EXIT), STATE_COMMAND_NO_RESP);
+ if (err) {
+ dev_err(irtoy->dev, "exit sample mode: %d\n", err);
+ return err;
+ }
+
+ err = irtoy_command(irtoy, COMMAND_SMODE_ENTER,
+ sizeof(COMMAND_SMODE_ENTER), STATE_COMMAND);
+ if (err) {
+ dev_err(irtoy->dev, "enter sample mode: %d\n", err);
+ return err;
+ }
+
err = irtoy_command(irtoy, COMMAND_TXSTART, sizeof(COMMAND_TXSTART),
STATE_TX);
kfree(buf);
--
2.31.1