Avi Kivity wrote:
> Zhang, Xiantao wrote:
>> Hi, Avi
>> Could you help to queue this patch for 2.6.28? It has no any logic
>> changes, and just re-organizes the structure of vm data area to allow
>> users to configure large memory and more vcpus. Since it changes
>> many in header files, so it had better go into rc1. Thanks!
>>
>
> If there are no logic changes, why is it needed to 2.6.28?
>
>> diff --git a/arch/ia64/include/asm/kvm.h
>> b/arch/ia64/include/asm/kvm.h index f38472a..de02c18 100644 ---
>> a/arch/ia64/include/asm/kvm.h +++ b/arch/ia64/include/asm/kvm.h
>> @@ -207,5 +207,4 @@ struct kvm_sregs {
>>
>> struct kvm_fpu {
>> };
>> -
>> #endif
>>
>
> Actually the blank line is helpful here.
Okay, keep it.
>> diff --git a/arch/ia64/include/asm/kvm_host.h
>> b/arch/ia64/include/asm/kvm_host.h
>> index b9e3c7f..e98f6f0 100644
>> --- a/arch/ia64/include/asm/kvm_host.h
>> +++ b/arch/ia64/include/asm/kvm_host.h
>> @@ -23,17 +23,6 @@
>> #ifndef __ASM_KVM_HOST_H
>> #define __ASM_KVM_HOST_H
>>
>> -
>> -#include <linux/types.h>
>> -#include <linux/mm.h>
>> -#include <linux/kvm.h>
>> -#include <linux/kvm_para.h>
>> -#include <linux/kvm_types.h>
>>
> Are you sure these are unneeded here? For example, if u8 or u32 are
> referenced in this file, you should keep linux/types.h. A header file
> should require no other header files to be previously included.
I didn't remove it, and just exclude the assembly code to include them. See below.
ifndef __ASSEMBLY__
+
+/*Define the max vcpus and memory for Guests.*/
+#define KVM_MAX_VCPUS (KVM_VM_DATA_SIZE - KVM_P2M_SIZE - KVM_VM_STRUCT_SIZE -\
+ KVM_MEM_DIRTY_LOG_SIZE) / sizeof(struct kvm_vcpu_data)
+#define KVM_MAX_MEM_SIZE (KVM_P2M_SIZE >> 3 << PAGE_SHIFT)
+
+#include <linux/types.h>
+#include <linux/mm.h>
+#include <linux/kvm.h>
+#include <linux/kvm_para.h>
+#include <linux/kvm_types.h>
+
+#include <asm/pal.h>
+#include <asm/sal.h>
+#include <asm/page.h>
>> @ -765,21 +763,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
>>
>> static void kvm_init_vm(struct kvm *kvm)
>> {
>> - long vm_base;
>> -
>> BUG_ON(!kvm);
>>
>> kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
>> kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
>> kvm->arch.vmm_init_rr = VMM_INIT_RR;
>>
>> - vm_base = kvm->arch.vm_base;
>> - if (vm_base) {
>> - kvm->arch.vhpt_base = vm_base + KVM_VHPT_OFS;
>> - kvm->arch.vtlb_base = vm_base + KVM_VTLB_OFS;
>> - kvm->arch.vpd_base = vm_base + KVM_VPD_OFS;
>> - }
>> -
>>
>
> Is this guaranteed to be zero? It doesn't seem like a code cleanup.
> Best in a separate patch with an explanation why it is safe.
These fields is useless and should be removed from the kvm_arch structure. I will remove them in next version.
>> @@ -183,8 +183,8 @@ void mark_pages_dirty(struct kvm_vcpu *v, u64
>> pte, u64 ps) u64 i, dirty_pages = 1;
>> u64 base_gfn = (pte&_PAGE_PPN_MASK) >> PAGE_SHIFT;
>> spinlock_t *lock = __kvm_va(v->arch.dirty_log_lock_pa);
>> - void *dirty_bitmap = (void *)v - (KVM_VCPU_OFS + v->vcpu_id *
>> VCPU_SIZE)
>> - + KVM_MEM_DIRTY_LOG_OFS;
>> + void *dirty_bitmap = (void *)KVM_MEM_DIRTY_LOG_BASE; +
>>
>
> This doesn't seem like a cleanup.
>
> In general the offsetof() changes are very good, but please separate
> them from the typos and coding style fixes.
Okay, I will separate them in next version. Thanks!
Xiantao
--
To unsubscribe from this list: send the line "unsubscribe kvm-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
