Re: [PATCH RFC] virtio-pci: share config interrupt between virtio devices
On Mon, Sep 01, 2014 at 01:41:54PM +0800, Amos Kong wrote:
> One VM only has 128 msix interrupt, virtio-config interrupt
> has less workload. This patch shares one normal interrupt
normal == INT#x? Please don't call it normal. The proper name is
"legacy INT#x".
So you are trying to use legacy INT#x at the same time
with MSI-X? This does not work: the PCI spec says:
While enabled for MSI or MSI-X
operation, a function is prohibited from using its INTx# pin (if
implemented) to request
service (MSI, MSI-X, and INTx# are mutually exclusive).
does the patch work for you? If it does it might be a (minor) spec
violation in kvm.
Besides, INT#x really leads to terrible performance because
sharing is forced even if there aren't many devices.
Why do we need INT#x?
How about setting IRQF_SHARED for the config interrupt
while using MSI-X? You'd have to read ISR to check that the interrupt was
intended for your device.
> for configuration between virtio devices.
>
> Signed-off-by: Amos Kong
> ---
> drivers/virtio/virtio_pci.c | 41 -
> 1 file changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index 3d1463c..b1263b3 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -52,6 +52,7 @@ struct virtio_pci_device
> /* Name strings for interrupts. This size should be enough,
>* and I'm too lazy to allocate each name separately. */
> char (*msix_names)[256];
> + char config_msix_name[256];
> /* Number of available vectors */
> unsigned msix_vectors;
> /* Vectors allocated, excluding per-vq vectors if any */
> @@ -282,12 +283,6 @@ static void vp_free_vectors(struct virtio_device *vdev)
> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>
> if (vp_dev->msix_enabled) {
> - /* Disable the vector used for configuration */
> - iowrite16(VIRTIO_MSI_NO_VECTOR,
> - vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
> - /* Flush the write out to device */
> - ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
> -
> pci_disable_msix(vp_dev->pci_dev);
> vp_dev->msix_enabled = 0;
> }
> @@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device
> *vdev, int nvectors,
> goto error;
> vp_dev->msix_enabled = 1;
>
> - /* Set the vector used for configuration */
> - v = vp_dev->msix_used_vectors;
> - snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
> + /* Set shared IRQ for configuration */
> + snprintf(vp_dev->config_msix_name, sizeof(*vp_dev->msix_names),
>"%s-config", name);
> - err = request_irq(vp_dev->msix_entries[v].vector,
> - vp_config_changed, 0, vp_dev->msix_names[v],
> + err = request_irq(vp_dev->pci_dev->irq,
> + vp_config_changed,
> + IRQF_SHARED,
> + vp_dev->config_msix_name,
> vp_dev);
> - if (err)
> - goto error;
> - ++vp_dev->msix_used_vectors;
> -
> - iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
> - /* Verify we had enough resources to assign the vector */
> - v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
> - if (v == VIRTIO_MSI_NO_VECTOR) {
> - err = -EBUSY;
> + if (!err)
> + vp_dev->intx_enabled = 1;
> + else
> goto error;
> - }
>
> if (!per_vq_vectors) {
> /* Shared vector for all VQs */
> @@ -535,14 +524,16 @@ static int vp_try_to_find_vqs(struct virtio_device
> *vdev, unsigned nvqs,
> goto error_request;
> } else {
> if (per_vq_vectors) {
> - /* Best option: one for change interrupt, one per vq. */
> - nvectors = 1;
> + /* Best option: one normal interrupt for change,
> +one msix per vq. */
> + nvectors = 0;
> for (i = 0; i < nvqs; ++i)
> if (callbacks[i])
> ++nvectors;
> } else {
> - /* Second best: one for change, shared for all vqs. */
> - nvectors = 2;
> + /* Second best: one normal interrupt for
> +change, share one msix for all vqs. */
> + nvectors = 1;
> }
>
> err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors);
> --
> 1.9.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC] virtio-pci: share config interrupt between virtio devices
One VM only has 128 msix interrupt, virtio-config interrupt
has less workload. This patch shares one normal interrupt
for configuration between virtio devices.
Signed-off-by: Amos Kong
---
drivers/virtio/virtio_pci.c | 41 -
1 file changed, 16 insertions(+), 25 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..b1263b3 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -52,6 +52,7 @@ struct virtio_pci_device
/* Name strings for interrupts. This size should be enough,
* and I'm too lazy to allocate each name separately. */
char (*msix_names)[256];
+ char config_msix_name[256];
/* Number of available vectors */
unsigned msix_vectors;
/* Vectors allocated, excluding per-vq vectors if any */
@@ -282,12 +283,6 @@ static void vp_free_vectors(struct virtio_device *vdev)
free_cpumask_var(vp_dev->msix_affinity_masks[i]);
if (vp_dev->msix_enabled) {
- /* Disable the vector used for configuration */
- iowrite16(VIRTIO_MSI_NO_VECTOR,
- vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
- /* Flush the write out to device */
- ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
-
pci_disable_msix(vp_dev->pci_dev);
vp_dev->msix_enabled = 0;
}
@@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device
*vdev, int nvectors,
goto error;
vp_dev->msix_enabled = 1;
- /* Set the vector used for configuration */
- v = vp_dev->msix_used_vectors;
- snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
+ /* Set shared IRQ for configuration */
+ snprintf(vp_dev->config_msix_name, sizeof(*vp_dev->msix_names),
"%s-config", name);
- err = request_irq(vp_dev->msix_entries[v].vector,
- vp_config_changed, 0, vp_dev->msix_names[v],
+ err = request_irq(vp_dev->pci_dev->irq,
+ vp_config_changed,
+ IRQF_SHARED,
+ vp_dev->config_msix_name,
vp_dev);
- if (err)
- goto error;
- ++vp_dev->msix_used_vectors;
-
- iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
- /* Verify we had enough resources to assign the vector */
- v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
- if (v == VIRTIO_MSI_NO_VECTOR) {
- err = -EBUSY;
+ if (!err)
+ vp_dev->intx_enabled = 1;
+ else
goto error;
- }
if (!per_vq_vectors) {
/* Shared vector for all VQs */
@@ -535,14 +524,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev,
unsigned nvqs,
goto error_request;
} else {
if (per_vq_vectors) {
- /* Best option: one for change interrupt, one per vq. */
- nvectors = 1;
+ /* Best option: one normal interrupt for change,
+ one msix per vq. */
+ nvectors = 0;
for (i = 0; i < nvqs; ++i)
if (callbacks[i])
++nvectors;
} else {
- /* Second best: one for change, shared for all vqs. */
- nvectors = 2;
+ /* Second best: one normal interrupt for
+ change, share one msix for all vqs. */
+ nvectors = 1;
}
err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors);
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/6] kvm, mem-hotplug: Do not pin ept identity pagetable and apic access page.
Hi Gleb, Would you please help to review these patches ? Thanks. On 08/27/2014 06:17 PM, Tang Chen wrote: ept identity pagetable and apic access page in kvm are pinned in memory. As a result, they cannot be migrated/hot-removed. But actually they don't need to be pinned in memory. [For ept identity page] Just do not pin it. When it is migrated, guest will be able to find the new page in the next ept violation. [For apic access page] The hpa of apic access page is stored in VMCS APIC_ACCESS_ADDR pointer. When apic access page is migrated, we update VMCS APIC_ACCESS_ADDR pointer for each vcpu in addition. NOTE: Tested with -cpu xxx,-x2apic option. But since nested vm pins some other pages in memory, if user uses nested vm, memory hot-remove will not work. Change log v3 -> v4: 1. The original patch 6 is now patch 5. ( by Jan Kiszka ) 2. The original patch 1 is now patch 6 since we should unpin apic access page at the very last moment. Tang Chen (6): kvm: Use APIC_DEFAULT_PHYS_BASE macro as the apic access page address. kvm: Remove ept_identity_pagetable from struct kvm_arch. kvm: Make init_rmode_identity_map() return 0 on success. kvm, mem-hotplug: Reload L1' apic access page on migration in vcpu_enter_guest(). kvm, mem-hotplug: Reload L1's apic access page on migration when L2 is running. kvm, mem-hotplug: Do not pin apic access page in memory. arch/x86/include/asm/kvm_host.h | 3 +- arch/x86/kvm/svm.c | 15 +- arch/x86/kvm/vmx.c | 103 +++- arch/x86/kvm/x86.c | 22 +++-- include/linux/kvm_host.h| 3 ++ virt/kvm/kvm_main.c | 30 +++- 6 files changed, 135 insertions(+), 41 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
On Fri, Aug 29, 2014 at 06:47:54PM +0200, Paolo Bonzini wrote:
>Il 19/08/2014 11:04, Wanpeng Li ha scritto:
>> Section 11.11.2.3 of the SDM mentions "All other bits in the
>> IA32_MTRR_PHYSBASEn
>> and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
>> general-protection exception(#GP) if software attempts to write to them".
>> This
>> patch do it in kvm.
>>
>> Signed-off-by: Wanpeng Li
>
>This breaks if the guest maxphyaddr is higher than the host's (which
>sometimes happens depending on your hardware and how QEMU is
>configured).
>
>You need to use cpuid_maxphyaddr, like this
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index a375dfc42f6a..916e89515210 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1726,7 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> int i;
>- u64 mask = 0;
>+ u64 mask;
>
> if (!msr_mtrr_valid(msr))
> return false;
>@@ -1750,8 +1750,7 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr,
>u64 data)
> /* variable MTRRs */
> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>
>- for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
>- mask |= (1ULL << i);
>+ mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
> if ((msr & 1) == 0) {
> /* MTRR base */
> if (!valid_mtrr_type(data & 0xff))
>
>
Got it, thanks Paolo and Jan.
Regards,
Wanpeng Li
>Jan, can you see if this patch fixes the SeaBIOS triple fault you reported?
>
>Paolo
>
>> ---
>> arch/x86/kvm/x86.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index fb3ea7a..b85da5f 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -1726,6 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
>> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>> {
>> int i;
>> +u64 mask = 0;
>>
>> if (!msr_mtrr_valid(msr))
>> return false;
>> @@ -1749,10 +1750,21 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32
>> msr, u64 data)
>> /* variable MTRRs */
>> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>>
>> -if ((msr & 1) == 0)
>> +for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
>> +mask |= (1ULL << i);
>> +if ((msr & 1) == 0) {
>> /* MTRR base */
>> -return valid_mtrr_type(data & 0xff);
>> -/* MTRR mask */
>> +if (!valid_mtrr_type(data & 0xff))
>> +return false;
>> + mask |= 0xf00;
>> +} else
>> +/* MTRR mask */
>> +mask |= 0x7ff;
>> +if (data & mask) {
>> +kvm_inject_gp(vcpu, 0);
>> +return false;
>> +}
>> +
>> return true;
>> }
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kvm-unit-test failures
Il 29/08/2014 23:05, Chris J Arges ha scritto: > And indeed there is a condition where matched && already_matched are > both true. In this case we don't zero or increment nr_vcpus_matched_tsc. > Incrementing nr_vcpus_matched_tsc in that last else clause allows the > test to pass; however this is identical to the logic before the patch. Can you please trace the test using trace-cmd (http://www.linux-kvm.org/page/Tracing) and send the output? Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
