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 (xiantao.zhang@xxxxxxxxx)
>> + *
>> + */
>> +
>> +#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-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
