Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Hollis Blanchard]

On Thu, 2007-12-13 at 20:36 +0100, Christian Ehrhardt wrote:
> @@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
> addr, uint8_t *buf,
>                      phys_ram_dirty[addr1 >> TARGET_PAGE_BITS] |>                          (0xff & ~CODE_DIRTY_FLAG);
>                  }
> -#ifdef __ia64__
> -		kvm_sync_icache((unsigned long)ptr, l);
> +#ifdef USE_KVM
> +		flush_icache_range((unsigned long)ptr, ((unsigned long)ptr)+l);
>  #endif
>              }
>          } else {

A comment to explain why the icache needs flushing only in the KVM case
would be useful. Other than that I'm fine with it.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Zhang, Xiantao]

>  #include "qemu-kvm.h"
>  #endif
>  #if defined(CONFIG_USER_ONLY)
> @@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
>                      addr, uint8_t *buf, phys_ram_dirty[addr1 >>
>                          TARGET_PAGE_BITS] |= (0xff &
>                  ~CODE_DIRTY_FLAG); }
> -#ifdef __ia64__
> -		kvm_sync_icache((unsigned long)ptr, l);
> +#ifdef USE_KVM
> +		flush_icache_range((unsigned long)ptr, ((unsigned
long)ptr)+l);

Are you sure ia64 implement this function for applications ? I didn't
find it at my side, so I write it. 

If not, maybe you can change the patch like the following :)

 in target-ia64/cpu.h

#define flush_icache_range(start, end)  kvm_sync_icache(start,
(end)-(start))

Or upgrade kvm_sync_icach to comply with flush_icache_range, and change
the name to flush_icache_range. 

Thank you!
Xiantao

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Christian Ehrhardt]

Zhang, Xiantao wrote:
>Christian Ehrhardt wrote:
<[...]
>> @@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
>>                      addr, uint8_t *buf, phys_ram_dirty[addr1 >>
>>                          TARGET_PAGE_BITS] |= (0xff &
>>                  ~CODE_DIRTY_FLAG); }
>> -#ifdef __ia64__
>> -		kvm_sync_icache((unsigned long)ptr, l);
>> +#ifdef USE_KVM
>> +		flush_icache_range((unsigned long)ptr, ((unsigned
> long)ptr)+l);
> 
> Are you sure ia64 implement this function for applications ? I didn't
> find it at my side, so I write it. 
What do you mean with "implement it for applications" ?
Take a look at dyngen.h - it starts with the comment "dyngen helpers" 
and contains a lot of static functions to use them where you need.
I don't see anything obvious that would prevent these functions from
being built and the file has no own include statements which may 
change that. Therefor the include "dyngen.h" in the USE_KVM case
should be enough to have this function available for
cpu_physical_memory_rw in exec.c where we need it.
[...]
> Xiantao

---

Hollis Blanchard wrote:
> A comment to explain why the icache needs flushing only in the KVM case
> would be useful. Other than that I'm fine with it.
> 
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
AFAIK Plain qemu does not directly execute guest code on the processor,
so the icache is not an issue for it.
Qemu itself has the flush_icache_range function only as helper for the
dynamic code generation.
But we may now write executable guest code with our intercepted mmio
handling that is directly executed when switching back to the guest
context, therefore we need that invalidation in the kvm case.

For the case that I'm overlooking something in plain qemu, so that it
might need it too I add qemu-devel@nongnu.org for comments from there,
but currently I think to have it in #ifdef USE_KVM is the right way.


P.S. Hollis did you mean you would like to see a comment in the code
where that call takes place?

-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt@linux.vnet.ibm.com
Ehrhardt@de.ibm.com

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Zhang, Xiantao]

Christian Ehrhardt wrote:
> Zhang, Xiantao wrote:
>> Christian Ehrhardt wrote:
> <[...]
>>> @@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
>>>                      addr, uint8_t *buf, phys_ram_dirty[addr1 >>
>>>                          TARGET_PAGE_BITS] |= (0xff &
>>>                  ~CODE_DIRTY_FLAG); }
>>> -#ifdef __ia64__
>>> -		kvm_sync_icache((unsigned long)ptr, l);
>>> +#ifdef USE_KVM
>>> +		flush_icache_range((unsigned long)ptr, ((unsigned
long)ptr)+l);
>> 
>> Are you sure ia64 implement this function for applications ? I didn't
>> find it at my side, so I write it.
> What do you mean with "implement it for applications" ?
> Take a look at dyngen.h - it starts with the comment "dyngen helpers"
> and contains a lot of static functions to use them where you need.
> I don't see anything obvious that would prevent these functions from
> being built and the file has no own include statements which may
> change that. Therefor the include "dyngen.h" in the USE_KVM case
> should be enough to have this function available for
> cpu_physical_memory_rw in exec.c where we need it.

Good! I didn't notice this definition. Originally, I think you mean it
is a library functions. :) 

> 
> Hollis Blanchard wrote:
>> A comment to explain why the icache needs flushing only in the KVM
>> case would be useful. Other than that I'm fine with it.
>> 
>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> AFAIK Plain qemu does not directly execute guest code on the
> processor, so the icache is not an issue for it.
> Qemu itself has the flush_icache_range function only as helper for the
> dynamic code generation.
> But we may now write executable guest code with our intercepted mmio
> handling that is directly executed when switching back to the guest
> context, therefore we need that invalidation in the kvm case.
> 
> For the case that I'm overlooking something in plain qemu, so that it
> might need it too I add qemu-devel@nongnu.org for comments from there,
> but currently I think to have it in #ifdef USE_KVM is the right way.
> 
> 
> P.S. Hollis did you mean you would like to see a comment in the code
> where that call takes place?


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Hollis Blanchard]

On Fri, 2007-12-14 at 10:07 +0100, Christian Ehrhardt wrote:
> 
> Hollis Blanchard wrote:
> > A comment to explain why the icache needs flushing only in the KVM
> case
> > would be useful. Other than that I'm fine with it.
> > 
> > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> AFAIK Plain qemu does not directly execute guest code on the
> processor,
> so the icache is not an issue for it.
> Qemu itself has the flush_icache_range function only as helper for the
> dynamic code generation.
> But we may now write executable guest code with our intercepted mmio
> handling that is directly executed when switching back to the guest
> context, therefore we need that invalidation in the kvm case.
> 
> For the case that I'm overlooking something in plain qemu, so that it
> might need it too I add qemu-devel@nongnu.org for comments from there,
> but currently I think to have it in #ifdef USE_KVM is the right way.
> 
> 
> P.S. Hollis did you mean you would like to see a comment in the code
> where that call takes place?

Yes! Hopefully much shorter than this email... :-P

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Christian Ehrhardt]

Hollis Blanchard wrote:
> On Fri, 2007-12-14 at 10:07 +0100, Christian Ehrhardt wrote:
>> Hollis Blanchard wrote:
>>> A comment to explain why the icache needs flushing only in the KVM
>> case
>>> would be useful. Other than that I'm fine with it.
>>>
>>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>> AFAIK Plain qemu does not directly execute guest code on the
>> processor,
>> so the icache is not an issue for it.
>> Qemu itself has the flush_icache_range function only as helper for the
>> dynamic code generation.
>> But we may now write executable guest code with our intercepted mmio
>> handling that is directly executed when switching back to the guest
>> context, therefore we need that invalidation in the kvm case.
>>
>> For the case that I'm overlooking something in plain qemu, so that it
>> might need it too I add qemu-devel@nongnu.org for comments from there,
>> but currently I think to have it in #ifdef USE_KVM is the right way.
>>
>>
>> P.S. Hollis did you mean you would like to see a comment in the code
>> where that call takes place?
> 
> Yes! Hopefully much shorter than this email... :-P
> 
comment added, rebased and resent together with a updated mmio
callback simplification patch - I hope I didn't overlook a response
to the mmio callback thread again this time ;-)

-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt@linux.vnet.ibm.com
Ehrhardt@de.ibm.com

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [kvm-devel] [PATCH][UPDATE] kvm-userspace: sync

[Avi Kivity]

Christian Ehrhardt wrote:
> Subject: [PATCH][UPDATE] kvm-userspace: sync icache for more architectures
> From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
>
> A ia64 patch introduced kvm_sync_icache within a ifdef __ia64__, but the 
> concept of split caches is not bound to ia64. This patch replaces the the 
> call to kvm_synch_icache by the flush_icache_range function
> that is already available in qemu for ia64 and ppc (noop for x86). 
> The call now depends on USE_KVM.
>
>   

Applied, thanks.

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


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel