From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@qumranet.com>
Subject: Re: [patch 4/4] KVM: allow direct access to PMTimer port
Date: Sun, 25 May 2008 13:04:40 +0300
Message-ID: <483939B8.5080304@qumranet.com>
References: <20080524234342.983197667@localhost.localdomain> <20080525000036.710955693@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Chris Wright <chrisw@redhat.com>, kvm-devel <kvm@vger.kernel.org>,
	Len Brown <lenb@kernel.org>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from bzq-179-150-194.static.bezeqint.net ([212.179.150.194]:39404
	"EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754320AbYEYKEp (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 25 May 2008 06:04:45 -0400
In-Reply-To: <20080525000036.710955693@localhost.localdomain>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Marcelo Tosatti wrote:
> There's not much point in exiting for pmtimer reads, since it runs at a
> fixed clock rate and its start value is undefined.
>
> The KVM-specific ioctl to read the counter from userspace is not nice
> though. Ideas?
>
> CC: Len Brown <lenb@kernel.org>
>
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index baf9607..f7e44d8 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -24,6 +24,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/highmem.h>
>  #include <linux/sched.h>
> +#include <linux/acpi_pmtmr.h>
>  
>  #include <asm/desc.h>
>  
> @@ -424,6 +425,10 @@ static __init int svm_hardware_setup(void)
>  	iopm_va = page_address(iopm_pages);
>  	memset(iopm_va, 0xff, PAGE_SIZE * (1 << IOPM_ALLOC_ORDER));
>  	clear_bit(0x80, iopm_va); /* allow direct access to PC debug port */
> +	if (pmtmr_ioport)
> +		for (r = 0; r < 4; r++)
> +			clear_bit(pmtmr_ioport+r, iopm_va);
> +            
>   

What if the port conflicts with a virtualized port?

>  	iopm_base = page_to_pfn(iopm_pages) << PAGE_SHIFT;
>  
>  	if (boot_cpu_has(X86_FEATURE_NX))
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index aaa99ed..4f77fd7 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -26,6 +26,7 @@
>  #include <linux/highmem.h>
>  #include <linux/sched.h>
>  #include <linux/moduleparam.h>
> +#include <linux/acpi_pmtmr.h>
>  
>  #include <asm/io.h>
>  #include <asm/desc.h>
> @@ -3261,6 +3262,10 @@ static int __init vmx_init(void)
>  	va = kmap(vmx_io_bitmap_a);
>  	memset(va, 0xff, PAGE_SIZE);
>  	clear_bit(0x80, va);
> +	if (pmtmr_ioport)
> +		for (r = 0; r < 4; r++)
> +			clear_bit(pmtmr_ioport+r, va);
> +
>   

Instead of duplicating the code, how about an kvm_x86_ops interface?

>  	kunmap(vmx_io_bitmap_a);
>  
>  	va = kmap(vmx_io_bitmap_b);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e537005..9769f52 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -27,6 +27,7 @@
>  #include <linux/module.h>
>  #include <linux/mman.h>
>  #include <linux/highmem.h>
> +#include <linux/acpi_pmtmr.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/msr.h>
> @@ -790,6 +791,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_PIT:
>  	case KVM_CAP_NOP_IO_DELAY:
>  	case KVM_CAP_MP_STATE:
> +	case KVM_CAP_GET_PMTIMER:
>  		r = 1;
>  		break;
>  	case KVM_CAP_VAPIC:
> @@ -1678,6 +1680,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  		r = 0;
>  		break;
>  	}
> +	case KVM_GET_PMTIMER: {
> +		struct kvm_pmtimer pmtmr;
> +		r = -ENODEV;
> +		if (!pmtmr_ioport)
> +			goto out;
> +		pmtmr.val = inl(pmtmr_ioport);
> +		r = -EFAULT;
> +		if (copy_to_user(argp, &pmtmr, sizeof pmtmr))
> +			goto out;
> +		r = 0;
> +		break;
> +	}
>  	default:
>   

This definitely doesn't belong in kvm.  /dev/pmtimer?



-- 
error compiling committee.c: too many arguments to function