Avi Kivity wrote:
> Zhang, Xiantao wrote:
>> Resend.
>> Avi,
>>      Please help to apply it, Thanks!
>> Xiantao
>> 
>> 
> 
> (sorry for the late review)
> 
>> Since this module will be reloated to an isolated address
>> space from host side, so kvm-intel can't call printk of host
>> kernel. This patch implements the printk function for kvm-intel
>> module, so it doesn't need to suffer no-printk pains.
>> 
>> 
>> +#define BYTES_PER_LOG 1024
>> +
>> +struct kvm_vmm_log {
>> +    spinlock_t log_lock;
>> +    unsigned long w_pointer;
>> +    unsigned long r_pointer;
>> +    char log_slot[VMM_LOG_SIZE/BYTES_PER_LOG][BYTES_PER_LOG - 1];
+};
>> +
>> 
> 
> 1024 bytes per line?  that's wasteful.
> 
> Why not variable size records?
It is a ring buffer, and will be recycled one by one. Anyway variable
size should be better, but needs more logic. 

>> 
>> +static void vcpu_print_vmm_log(void)
>> +{
>> +    unsigned int slot;
>> +
>> +    spin_lock(&vmm_log->log_lock);
>> 
> 
> You're going to impact scalability with this.  Are per-vcpu logs
> workable? 

OK, I will change it to per-vcpu style to avoid this possible
scalability issue. 

>> @@ -1011,6 +1030,7 @@ int kvm_arch_vcpu_ioctl_translate(struct
>> kvm_vcpu *vcpu, 
>> 
>>  static int kvm_alloc_vmm_area(void)
>>  {
>> +
>> 
> 
> blank line?
Should be removed. 
>> diff --git a/arch/ia64/kvm/kvm_lib.c b/arch/ia64/kvm/kvm_lib.c
>> new file mode 100644
>> index 0000000..4c43efe
>> --- /dev/null
>> +++ b/arch/ia64/kvm/kvm_lib.c
>> @@ -0,0 +1,13 @@
>> +
>> +/*
>> + * vsprintf.c: Let kvm-intel module has the ability to use print
>> functions. + *       Just include kernel's library, and disable
symbols
>> export. + *  Copyright (C) 2008, Intel Corporation.
>> + *          Xiantao Zhang  ([EMAIL PROTECTED])
>> + *
>> + */
>> +
>> +#undef CONFIG_MODULES
>> +
>> +#include "../../../lib/vsprintf.c"
>> +#include "../../../lib/ctype.c"
>> 
> 
> I suspect this will start breaking when people start using the new
> printk("%pBLAH") functionality, which will require linking additional
> files. 

 If the format string works with vsnprintf, it should be covered. 

> Is it possible to pass the format string and the arguments in the log
> record instead, and do the printk() in the kernel?


> I can't think of a way on x86, but maybe ia64 varargs are different.
> 
> (worst case you can limit the number of arguments and just copy a
> bunch  of stack).

That maybe infeasible, since some args may be transferred by pointer,
and this pointer can't be reached in host side.
Xiantao

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to