Re: [patch 4/4] KVM: allow direct access to PMTimer port

[Marcelo Tosatti <mtosatti@redhat.com>]

On Sun, May 25, 2008 at 01:04:40PM +0300, Avi Kivity wrote:
> >@@ -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?

Userspace registers the port range in QEMU even if it can be accessed
directly, so if there is a conflict it can be handled there.

> >+	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?

Yes sounds much better.

> >+	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?

Think so.