Re: [PATCH 2/3] Add HYPER-V apic access MSRs.

[Avi Kivity <avi@redhat.com>]

On 01/13/2010 04:40 PM, Gleb Natapov wrote:
>
>>> +int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
>>> +{
>>> +	struct kvm_lapic *apic = vcpu->arch.apic;
>>> +	u32 reg = kvm_hv_vapic_msr2reg(msr), low, high = 0;
>>> +
>>> +	if (!irqchip_in_kernel(vcpu->kvm))
>>> +		return 1;
>>> +
>>> +	if (apic_reg_read(apic, reg, 4,&low))
>>> +		return 1;
>>> +	if (reg == APIC_ICR)
>>> +		apic_reg_read(apic, APIC_ICR2, 4,&high);
>>> +
>>> +	*data = (((u64)high)<<   32) | low;
>>> +
>>> +	return 0;
>>> +}
>>>        
>> I think it's cleaner to put all this into the (set_msr and get_msr).
>> You're just converting register numbers back and forth, while they
>> are known at the call site.
>>
>>      
> I thought about it. It will bring apic internals into x86.c. If you are
> OK with that I'll do it like you say.
>    

I think it's better than that tangle.
>> Space after if.
>>      
> Forgot about checkpatch again :(
>
>    

Should be hardwired into your neurons by now...

>>> +		addr = gfn_to_hva(vcpu->kvm, data>>
>>> +				  HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT);
>>> +		if (kvm_is_error_hva(addr))
>>> +			return 1;
>>>        
>> But the spec actually permits addresses outside RAM?
>>      
> Is says "in GPA space". Not sure how to interpret it. The guest I run
> uses address in real RAM.
>    

My interpretation is that it can be anywhere.  But that will be 
difficult, let's stick with this and hope we don't run into trouble.

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