Re: [Qemu-devel] [PATCH 28/31] vga: disable chain4_alias if KVM supports SMRAM

[Paolo Bonzini]

This actually is not needed anymore in the latest incarnation of the
kernel-side interface.

Paolo

On 11/05/2015 15:49, Paolo Bonzini wrote:
> KVM is okay with SMRAM overlapping an MMIO area underneath it, but it does
> not want to have two overlapping RAM slots for SMRAM and video RAM.
> Unfortunately, the chain4_alias optimization results in the latter
> situation.  Disable it if KVM supports system management mode.
> 
> Note that the chain4_alias optimization is misguided, because it assumes
> that chain4 data is at VRAM address 0,1,2,3,4,5,6,7...16383.  This is
> incorrect, because chain4 data is at VRAM address 0,1,2,3, 16,17,18,19,
> ..., 65523.  But we cannot fix the VRAM format without breaking migration,
> so keep the optimization and just disable it if it gets in the way.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/display/vga.c     | 8 ++++++--
>  hw/display/vga_int.h | 1 +
>  include/sysemu/kvm.h | 1 +
>  kvm-all.c            | 5 +++++
>  kvm-stub.c           | 5 +++++
>  5 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index d1d296c..b1925a0 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -154,7 +154,8 @@ static void vga_update_memory_access(VGACommonState *s)
>          s->has_chain4_alias = false;
>          s->plane_updated = 0xf;
>      }
> -    if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
> +    if (s->use_chain4_alias &&
> +     (s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
>          VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] & VGA_SR04_CHN_4M) 
> {
>          offset = 0;
>          switch ((s->gr[VGA_GFX_MISC] >> 2) & 3) {
> @@ -2219,8 +2220,11 @@ void vga_init(VGACommonState *s, Object *obj, 
> MemoryRegion *address_space,
>  
>      qemu_register_reset(vga_reset, s);
>  
> -    s->bank_offset = 0;
> +    if (!kvm_enabled() || !kvm_has_smm()) {
> +        s->use_chain4_alias = true;
> +    }
>  
> +    s->bank_offset = 0;
>      s->legacy_address_space = address_space;
>  
>      vga_io_memory = vga_init_io(s, obj, &vga_ports, &vbe_ports);
> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> index fcfcc5f..646e64c 100644
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -95,6 +95,7 @@ typedef struct VGACommonState {
>      uint32_t vram_size_mb; /* property */
>      uint32_t vbe_size;
>      uint32_t latch;
> +    bool use_chain4_alias;
>      bool has_chain4_alias;
>      MemoryRegion chain4_alias;
>      uint8_t sr_index;
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 4878959..e1db979 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -186,6 +186,7 @@ int kvm_has_pit_state2(void);
>  int kvm_has_many_ioeventfds(void);
>  int kvm_has_gsi_routing(void);
>  int kvm_has_intx_set_mask(void);
> +int kvm_has_smm(void);
>  
>  int kvm_init_vcpu(CPUState *cpu);
>  int kvm_cpu_exec(CPUState *cpu);
> diff --git a/kvm-all.c b/kvm-all.c
> index 17a3771..5ad4877 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1990,6 +1990,11 @@ int kvm_vm_check_attr(KVMState *s, uint32_t group, 
> uint64_t attr)
>      return ret ? 0 : 1;
>  }
>  
> +int kvm_has_smm(void)
> +{
> +    return kvm_check_extension(kvm_state, KVM_CAP_X86_SMM);
> +}
> +
>  int kvm_has_sync_mmu(void)
>  {
>      return kvm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 7ba90c5..c3428a5 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -56,6 +56,11 @@ int kvm_cpu_exec(CPUState *cpu)
>      abort();
>  }
>  
> +int kvm_has_smm(void)
> +{
> +    return 0;
> +}
> +
>  int kvm_has_sync_mmu(void)
>  {
>      return 0;
>