Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Dmitry Eremin-Solenikov]

Glauber Costa wrote:

> Remove explicit calls to mmio coalescing. Rather, include it in the
> registration functions.
> 

OK. On real SVM HW this seems to work. However now i'm stumbled upon another
problem wrt. NMI. See another mail.

-- 
With best wishes
Dmitry

se

--
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 8/9] coalesce mmio regions with an explicit call

[Glauber Costa]

On Thu, Sep 25, 2008 at 10:08:53AM +0300, Avi Kivity wrote:
> Glauber Costa wrote:
> >
> > Any ideas about what's up for the other hypervisors that may (we hope) be 
> > integrated
> > in the future? Xen?
> >   
> 
> Xen should benefit even more (much more).  IIRC Windows wouldn't boot
> since it was spending all its time context switching when the splash
> screen with its KITT bar was displayed, so they hacked something for
> vga, but nothing generic.
That's the point. It's not just a word play between qemu and kvm, because
if we introduce generic hooks that kvm happens to fill, but qemu not, other 
hypervirors
may (we hope) fill it in the future.

--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Avi Kivity]

Glauber Costa wrote:
>
> Any ideas about what's up for the other hypervisors that may (we hope) be 
> integrated
> in the future? Xen?
>   

Xen should benefit even more (much more).  IIRC Windows wouldn't boot
since it was spending all its time context switching when the splash
screen with its KITT bar was displayed, so they hacked something for
vga, but nothing generic.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Glauber Costa]

On Wed, Sep 24, 2008 at 02:10:26PM +0300, Avi Kivity wrote:
> Glauber Costa wrote:
>>> You can't coalesce the registers which trigger device action.  You'll 
>>>  destroy latency and/or functionality.
>>> 
>>
>> which kills the goal of getting rid of explicit kvm code.
>>
>>   
>
> It's a fact that coalescing helps kvm but not qemu.
>
>> So maybe the solution here is to add calls in qemu to a memory
>> coalescing function that in the raw qemu / kqemu case just don't
>> do anything?
>>   
>
> That's just word games.  s/kvm/qemu/ won't change the fact that this is  
> a kvm specific hook.
Any ideas about what's up for the other hypervisors that may (we hope) be 
integrated
in the future? Xen?

--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Avi Kivity]

Glauber Costa wrote:
You can't coalesce the registers which trigger device action.  You'll  
destroy latency and/or functionality.



which kills the goal of getting rid of explicit kvm code.

  


It's a fact that coalescing helps kvm but not qemu.


So maybe the solution here is to add calls in qemu to a memory
coalescing function that in the raw qemu / kqemu case just don't
do anything?
  


That's just word games.  s/kvm/qemu/ won't change the fact that this is 
a kvm specific hook.


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

--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Glauber Costa]

On Tue, Sep 23, 2008 at 10:29:48AM +0300, Avi Kivity wrote:
> Glauber Costa wrote:
>> On Sat, Sep 20, 2008 at 11:39:44AM -0700, Avi Kivity wrote:
>>   
>>> Glauber Costa wrote:
>>> 
 Remove explicit calls to mmio coalescing. Rather,
 include it in the registration functions.

 index 5ae3960..2d97b34 100644
 --- a/qemu/hw/e1000.c
 +++ b/qemu/hw/e1000.c
 @@ -942,18 +942,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
   d->mmio_base = addr;
  cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
 -
 -if (kvm_enabled()) {
 -  int i;
 -uint32_t excluded_regs[] = {
 -E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
 -E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
 -};
 -qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
 -for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
 -qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
 - excluded_regs[i + 1] - excluded_regs[i] - 4);
 -}
  }
  
>>> Where did all of this go?
>>> 
>>
>> All the region is coalesced (not just the pieces) automatically during 
>> memory registration.
>> Or not at all, in case coalescing is disabled.
>>   
>
> You can't coalesce the registers which trigger device action.  You'll  
> destroy latency and/or functionality.

which kills the goal of getting rid of explicit kvm code.

So maybe the solution here is to add calls in qemu to a memory
coalescing function that in the raw qemu / kqemu case just don't
do anything?

Anthony, do you have an opinion about it ?
--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Avi Kivity]

Glauber Costa wrote:

On Sat, Sep 20, 2008 at 11:39:44AM -0700, Avi Kivity wrote:
  

Glauber Costa wrote:


Remove explicit calls to mmio coalescing. Rather,
include it in the registration functions.

index 5ae3960..2d97b34 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -942,18 +942,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
  d->mmio_base = addr;
 cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
-
-if (kvm_enabled()) {
-   int i;
-uint32_t excluded_regs[] = {
-E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
-E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
-};
-qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
-for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
-qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
- excluded_regs[i + 1] - excluded_regs[i] - 4);
-}
 }
   
  

Where did all of this go?



All the region is coalesced (not just the pieces) automatically during memory 
registration.
Or not at all, in case coalescing is disabled.
  


You can't coalesce the registers which trigger device action.  You'll 
destroy latency and/or functionality.


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

--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Glauber Costa]

On Sat, Sep 20, 2008 at 11:39:44AM -0700, Avi Kivity wrote:
> Glauber Costa wrote:
>> Remove explicit calls to mmio coalescing. Rather,
>> include it in the registration functions.
>>
>> index 5ae3960..2d97b34 100644
>> --- a/qemu/hw/e1000.c
>> +++ b/qemu/hw/e1000.c
>> @@ -942,18 +942,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
>>   d->mmio_base = addr;
>>  cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
>> -
>> -if (kvm_enabled()) {
>> -int i;
>> -uint32_t excluded_regs[] = {
>> -E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
>> -E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
>> -};
>> -qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
>> -for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
>> -qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
>> - excluded_regs[i + 1] - excluded_regs[i] - 4);
>> -}
>>  }
>>
>
> Where did all of this go?

All the region is coalesced (not just the pieces) automatically during memory 
registration.
Or not at all, in case coalescing is disabled.

>
> -- 
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
--
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

Re: [PATCH 8/9] coalesce mmio regions with an explicit call

[Avi Kivity]

Glauber Costa wrote:

Remove explicit calls to mmio coalescing. Rather,
include it in the registration functions.

index 5ae3960..2d97b34 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -942,18 +942,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
 
 d->mmio_base = addr;

 cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
-
-if (kvm_enabled()) {
-   int i;
-uint32_t excluded_regs[] = {
-E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
-E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
-};
-qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
-for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
-qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
- excluded_regs[i + 1] - excluded_regs[i] - 4);
-}
 }
 
  


Where did all of this go?

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

--
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

[PATCH 8/9] coalesce mmio regions with an explicit call

[Glauber Costa]

Remove explicit calls to mmio coalescing. Rather,
include it in the registration functions.

Signed-off-by: Glauber Costa <[EMAIL PROTECTED]>
---
 qemu/hw/cirrus_vga.c |2 --
 qemu/hw/e1000.c  |   12 
 qemu/hw/pci.c|3 ---
 qemu/hw/vga.c|4 
 qemu/qemu-kvm.c  |   10 +-
 qemu/qemu-kvm.h  |4 
 6 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
index 0cf5b24..5919732 100644
--- a/qemu/hw/cirrus_vga.c
+++ b/qemu/hw/cirrus_vga.c
@@ -3291,8 +3291,6 @@ static void cirrus_init_common(CirrusVGAState * s, int 
device_id, int is_pci)
cirrus_vga_mem_write, s);
 cpu_register_physical_memory(isa_mem_base + 0x000a, 0x2,
  vga_io_memory);
-if (kvm_enabled())
-qemu_kvm_register_coalesced_mmio(isa_mem_base + 0x000a, 0x2);
 
 s->sr[0x06] = 0x0f;
 if (device_id == CIRRUS_ID_CLGD5446) {
diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
index 5ae3960..2d97b34 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -942,18 +942,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
 
 d->mmio_base = addr;
 cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
-
-if (kvm_enabled()) {
-   int i;
-uint32_t excluded_regs[] = {
-E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
-E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
-};
-qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
-for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
-qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
- excluded_regs[i + 1] - excluded_regs[i] - 4);
-}
 }
 
 static int
diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index 07d37a8..2e4ec92 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -324,9 +324,6 @@ static void pci_update_mappings(PCIDevice *d)
 cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
  r->size,
  IO_MEM_UNASSIGNED);
-if (kvm_enabled())
-qemu_kvm_unregister_coalesced_mmio(r->addr,
- r->size);
 }
 }
 r->addr = new_addr;
diff --git a/qemu/hw/vga.c b/qemu/hw/vga.c
index 3a5dcbc..ba0dec4 100644
--- a/qemu/hw/vga.c
+++ b/qemu/hw/vga.c
@@ -2259,8 +2259,6 @@ void vga_init(VGAState *s)
 vga_io_memory = cpu_register_io_memory(0, vga_mem_read, vga_mem_write, s);
 cpu_register_physical_memory(isa_mem_base + 0x000a, 0x2,
  vga_io_memory);
-if (kvm_enabled())
-qemu_kvm_register_coalesced_mmio(isa_mem_base + 0x000a, 0x2);
 }
 
 /* Memory mapped interface */
@@ -2336,8 +2334,6 @@ static void vga_mm_init(VGAState *s, target_phys_addr_t 
vram_base,
 cpu_register_physical_memory(ctrl_base, 0x10, s_ioport_ctrl);
 s->bank_offset = 0;
 cpu_register_physical_memory(vram_base + 0x000a, 0x2, 
vga_io_memory);
-if (kvm_enabled())
-qemu_kvm_register_coalesced_mmio(vram_base + 0x000a, 0x2);
 }
 
 int isa_vga_init(DisplayState *ds, uint8_t *vga_ram_base,
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 721a9dc..660e11f 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -795,6 +795,7 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t 
start_addr,
 printf("No free mmio slots\n");
 exit(1);
 }
+kvm_register_coalesced_mmio(kvm_context, start_addr, size);
 return;
 }
 
@@ -1039,12 +1040,3 @@ void kvm_mutex_lock(void)
 pthread_mutex_lock(&qemu_mutex);
 cpu_single_env = NULL;
 }
-
-int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr, unsigned int 
size)
-{
-}
-
-int qemu_kvm_unregister_coalesced_mmio(target_phys_addr_t addr,
-  unsigned int size)
-{
-}
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 3e40a7d..4308e18 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -75,10 +75,6 @@ int handle_tpr_access(void *opaque, int vcpu,
 void kvm_tpr_vcpu_start(CPUState *env);
 
 int qemu_kvm_get_dirty_pages(unsigned long phys_addr, void *buf);
-int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr,
-unsigned int size);
-int qemu_kvm_unregister_coalesced_mmio(target_phys_addr_t addr,
-  unsigned int size);
 
 void qemu_kvm_system_reset_request(void);
 
-- 
1.5.5.1

--
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