Re: [PATCH 2/3 v2] x2APIC interface to local apic

[Avi Kivity <avi@redhat.com>]

On 06/28/2009 03:15 PM, Gleb Natapov wrote:
> This patch implements MSR interface to a local apic as defines by x2APIC
> Intel specification.
>
> @@ -269,7 +275,12 @@ int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
>   int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
>   {
>   	int result = 0;
> -	u8 logical_id;
> +	u32 logical_id;
> +
> +	if (apic_x2apic_mode(apic)) {
> +		logical_id = apic_get_reg(apic, APIC_LDR);
> +		return logical_id&  (uint16_t)mda;
> +	}
>    

mda is u8, why cast it?

> +static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
> +		void *data)
>   {
> -	struct kvm_lapic *apic = to_lapic(this);
> -	unsigned int offset = address - apic->base_address;
>   	unsigned char alignment = offset&  0xf;
>   	u32 result;
> +	/* this bitmask has a bit cleared for each reserver register */
> +	static const uint64_t rmask = 0x43ff01ffffffe70c;
>    

s/uint64_t/u64/, add ULL to avoid warnings on i386?

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5a66bb9..8ead54d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -817,6 +817,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>   	case MSR_IA32_APICBASE:
>   		kvm_set_apic_base(vcpu, data);
>   		break;
> +	case APIC_BASE_MSR ... APIC_BASE_MSR + 0x3ff:
> +		return kvm_x2apic_msr_write(vcpu, msr, data);
>    

Might be a good idea to special cases these in arch code to avoid the 
long if-trees the switches generate.  Only after profiling though.

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