Re: NMI Injection to Guest

2009-08-01 Thread Jiaqing Du
Hi Gleb,

Another problem on AMD processors.

After each vm-exit, I need to check if this vm-exit is due to NMI. For
vmx.c, I add the check in vmx_complete_interrupts().

The code snippet is:

3539 if ((exit_intr_info & INTR_INFO_INTR_TYPE_MASK) ==
INTR_TYPE_NMI_INTR &&
3540 (exit_intr_info & INTR_INFO_VALID_MASK)) {
3541
3542 printk(KERN_INFO "kvm-oprofile: vm exit due to NMI.\n");
3543
3544 /* indicate vm-exit due to conter overflow */
3545 vcpu->vm_exit_on_cntr_overflow = 1;
3546 }

This works on Intel chips.

I did the similar check in svm_complete_interrupts().

2501 static void svm_complete_interrupts(struct vcpu_svm *svm)
2502 {
2503 u8 vector;
2504 int type;
2505 u32 exitintinfo = svm->vmcb->control.exit_int_info;
2506 struct kvm_vcpu *vcpu = &svm->vcpu;
2507
2508 if (svm->vcpu.arch.hflags & HF_IRET_MASK)
2509 svm->vcpu.arch.hflags &= ~(HF_NMI_MASK | HF_IRET_MASK);
2510
2511 svm->vcpu.arch.nmi_injected = false;
2512 kvm_clear_exception_queue(&svm->vcpu);
2513 kvm_clear_interrupt_queue(&svm->vcpu);
2514
2515 if (!(exitintinfo & SVM_EXITINTINFO_VALID))
2516 return;
2517
2518 vector = exitintinfo & SVM_EXITINTINFO_VEC_MASK;
2519 type = exitintinfo & SVM_EXITINTINFO_TYPE_MASK;
2520
2521 /* kvm-oprofile */
2522 if (type == SVM_EXITINTINFO_TYPE_NMI) {
2523
2524 printk(KERN_INFO "kvm-oprofile:
counter_overflowed & vm exit.\n");
2525 vcpu->vm_exit_on_cntr_overflow = 1;
2526 }

However, this part (2522 to 2526) never got executed. By using qemu
monitor, I managed to inject NMI to the guests. But this check, after
vm-exit due to NMI, does not succeed.


Thanks,
Jiaqing

2009/7/30 Jiaqing Du :
> Hi Gleb,
>
> My code works by setting "vcpu->arch.nmi_pending = 1;" inside
> vcpu_enter_guest().
>
>
> Thanks,
> Jiaqing
>
> 2009/7/27 Gleb Natapov :
>> On Sun, Jul 26, 2009 at 09:25:34PM +0200, Jiaqing Du wrote:
>>> Hi Gleb,
>>>
>>> Thanks for your reply.
>>>
>>> 2009/7/26 Gleb Natapov :
>>> > On Sat, Jul 25, 2009 at 10:46:39PM +0200, Jiaqing Du wrote:
>>> >> Hi list,
>>> >>
>>> >> I'm trying to extend OProfile to support guest profiling. One step of
>>> >> my work is to push an NMI to the guest(s) when a performance counter
>>> >> overflows. Please correct me if the following is not correct:
>>> >>
>>> >> counter overflow --> NMI to host --> VM exit --> "int $2" to handle
>>> >> NMI on host --> ...   --> VM entry --> NMI to guest
>>> >>
>>> > Correct except the last step (--> NMI to guest). Host nmi is not
>>> > propagated to guests.
>>>
>>> Yes. I need to add some code to propagate host NMI to guests.
>>> >
>>> >> On the path between VM-exit and VM-entry, I want to push an NMI to the
>>> >> guest. I tried to put the following code on the path, but never
>>> >> succeeded. Various wired things happened, such as KVM hangs, guest
>>> >> kernel oops, and host hangs. I tried both code with Linux 2.6.30 and
>>> >> version 88.
>>> >>
>>> >> if (vmx_nmi_allowed())  { vmx_inject_nmi(); }
>>> >>
>>> >> Any suggestions? Where is the right place to push an NMI and what are
>>> >> the necessary checks?
>>> > Call kvm_inject_nmi(vcpu). And don't forget to vcpu_load(vcpu) before
>>> > doing it. See kvm_vcpu_ioctl_nmi().
>>>
>>> Based on the code with Linux 2.6.30, what kvm_inject_nmi(vcpu) does is
>>> just set vcpu->arch.nmi_pending to 1. kvm_vcpu_ioctl_nmi() puts
>>> vcpu_load() before the setting and vcpu_put() after it.
>>>
>>> I need to push host NMI to guests between a VM-exit and a VM-entry
>>> after that. The VM-exit is due to an NMI caused by performance counter
>>> overflow. The following code with vcpu_enter_guest(), which is
>>> surrounded by a vcpu_load() and vcpu_put(), checks this
>>> vcpu->arch.nmi_pending and other related flags to decide whether an
>>> NMI should be pushed to guests.
>>>
>>>       if (vcpu->arch.exception.pending)
>>>               __queue_exception(vcpu);
>>>       else if (irqchip_in_kernel(vcpu->kvm))
>>>               kvm_x86_ops->inject_pending_irq(vcpu);
>>>       else
>>>               kvm_x86_ops->inject_pending_vectors(vcpu, kvm_run);
>>>
>>> What I did is given below:
>>>
>>> 3097 static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run 
>>> *kvm_run)
>>> 3098 {
>>>                ... ...
>>>
>>> 3156         if (kvm_vm_exit_on_cnt_overflow) {
>>> 3157                 vcpu->arch.nmi_pending = 1;
>>> 3158         }
>>> 3159
>>> 3160         if (vcpu->arch.exception.pending)
>>> 3161                 __queue_exception(vcpu);
>>> 3162         else if (irqchip_in_kernel(vcpu->kvm))
>>> 3163                 kvm_x86_ops->inject_pending_irq(vcpu);
>>> 3164         else
>>> 3165                 kvm_x86_ops->inject_pending_vectors(vcpu, kvm_run);
>>>
>>>               ... 
>>> 3236 }
>>>
>>> In vcpu_enter_guest(), before this part of cod

Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread SAL
On Sat, Aug 01, 2009 at 10:19:20AM +0200, Alexander Graf wrote:
>
> On 01.08.2009, at 10:01, Ján ONDREJ (SAL) wrote:
>
>> On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:
>>> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:

 On 31.07.2009, at 20:19, Glauber Costa  wrote:

> cp "$1" "$2"
> +sum=$(echo "obase=8; $sum" | bc)
> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 
> 2>/
> dev/null
>>>
>>> May be it's better to use awk:
>>>
>>> echo $sum | awk '{ printf("%c", $1) }' \
>>>  | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
>>>
>>> This version accepts decimal $sum, does not need conversion to octal.
>>
>> Ane may be it's better to write whole checksum in awk, it's faster and
>> cleaner. See attached patch (I am sorry, I don't know how to use git- 
>> email).
>
> My awk magic is pretty bad, but I think this version is missing the  
> padding, right?

Right. If you prefer this version agains Paolos double printf, I can
update it, if not, just use Pauols solution.

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


Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread Alexander Graf


On 01.08.2009, at 01:39, Glauber Costa wrote:


On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:


On 31.07.2009, at 20:19, Glauber Costa  wrote:


Since commit 89e671e3, extboot is broken due to wrong checksum

The problem is that printf "\\$sum" syntax will require an octal
representation, so the fix I'm proposing is to convert it first.


Is there no easy way to tell printf we're on decimal? I don't have a
Linux system handy atm, but I thought \90 was in fact a 90.

Either way, my only complaint would be to introduce a dependency on  
bc.

Not that I'm aware of.
But would be happy to know, too.


Hum, reading the documentation again it in fact needs an octal number.
But we could use printf for the conversion as well!

$ printf "%o" 65
101

That wouldn't introduce a new dependency that might be missing on  
random OSs and looks rather clean to me.


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


Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread Paolo Bonzini

On 07/31/2009 09:55 PM, Alexander Graf wrote:


On 31.07.2009, at 20:19, Glauber Costa  wrote:


Since commit 89e671e3, extboot is broken due to wrong checksum

The problem is that printf "\\$sum" syntax will require an octal
representation, so the fix I'm proposing is to convert it first.


Is there no easy way to tell printf we're on decimal? I don't have a
Linux system handy atm, but I thought \90 was in fact a 90.

Either way, my only complaint would be to introduce a dependency on bc.


printf `printf '\\%o' $sum`

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


Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread Alexander Graf


On 01.08.2009, at 10:01, Ján ONDREJ (SAL) wrote:


On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:

On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:


On 31.07.2009, at 20:19, Glauber Costa  wrote:


cp "$1" "$2"
+sum=$(echo "obase=8; $sum" | bc)
printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc  
2>/

dev/null


May be it's better to use awk:

echo $sum | awk '{ printf("%c", $1) }' \
 | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null

This version accepts decimal $sum, does not need conversion to octal.


Ane may be it's better to write whole checksum in awk, it's faster and
cleaner. See attached patch (I am sorry, I don't know how to use git- 
email).


My awk magic is pretty bad, but I think this version is missing the  
padding, right?


Alex

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


Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread SAL
On Sat, Aug 01, 2009 at 09:04:55AM +0200, Ján ONDREJ (SAL) wrote:
> On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
> >
> > On 31.07.2009, at 20:19, Glauber Costa  wrote:
> >
> >> cp "$1" "$2"
> >> +sum=$(echo "obase=8; $sum" | bc)
> >> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/ 
> >> dev/null
> 
> May be it's better to use awk:
> 
> echo $sum | awk '{ printf("%c", $1) }' \
>   | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
> 
> This version accepts decimal $sum, does not need conversion to octal.

Ane may be it's better to write whole checksum in awk, it's faster and
cleaner. See attached patch (I am sorry, I don't know how to use git-email).

SAL
diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
index 4273d1f..065b6a9 100755
--- a/pc-bios/optionrom/signrom.sh
+++ b/pc-bios/optionrom/signrom.sh
@@ -18,28 +18,31 @@
 #
 # Copyright Novell Inc, 2009
 #   Authors: Alexander Graf 
+#Jan Ondrej (SAL) 
 #
 # Syntax: signrom.sh  
 
 # did we get proper arguments?
 test "$1" -a "$2" || exit 1
 
-sum=0
-
-# find out the file size
-x=`dd if="$1" bs=1 count=1 skip=2 2>/dev/null | od -t u1 -A n`
-#size=`expr $x \* 512 - 1`
-size=$(( $x * 512 - 1 ))
-
-# now get the checksum
-for i in `od -A n -t u1 -v "$1"`; do
-# add each byte's value to sum
-sum=$(( $sum + $i ))
-done
-
-sum=$(( $sum % 256 ))
-sum=$(( 256 - $sum ))
-
-# and write the output file
-cp "$1" "$2"
-printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
+od -A n -t u1 -v "$1" | awk '
+BEGIN {
+checksum=0
+# last byte will be replaced by checksum
+last=-1
+}
+{
+# go over all record in row
+for(i=1; i<=NF; i++) {
+checksum+=$i
+if (last>=0) {
+printf "%c", last
+}
+last=$i
+}
+}
+END {
+# compute checksum
+printf "%c", 256-(checksum%256)
+}
+' > $2


Re: [PATCH] unbreak booting with virtio

2009-08-01 Thread SAL
On Fri, Jul 31, 2009 at 09:55:39PM +0200, Alexander Graf wrote:
>
> On 31.07.2009, at 20:19, Glauber Costa  wrote:
>
>> Since commit 89e671e3, extboot is broken due to wrong checksum
>>
>> The problem is that printf "\\$sum" syntax will require an octal
>> representation, so the fix I'm proposing is to convert it first.
>
> Is there no easy way to tell printf we're on decimal? I don't have a  
> Linux system handy atm, but I thought \90 was in fact a 90.
>
> Either way, my only complaint would be to introduce a dependency on bc.

>> cp "$1" "$2"
>> +sum=$(echo "obase=8; $sum" | bc)
>> printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/ 
>> dev/null

May be it's better to use awk:

echo $sum | awk '{ printf("%c", $1) }' \
  | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null

This version accepts decimal $sum, does not need conversion to octal.

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