Re: [Qemu-devel] [PATCH v2 17/23] q35: implement high SMRAM

[Michael S. Tsirkin]

On Wed, Jun 03, 2015 at 07:08:42PM +0200, Paolo Bonzini wrote:
> When H_SMRAME is 1, low memory at 0xa0000 is left alone by
> SMM, and instead the chipset maps the 0xa0000-0xbffff window at
> 0xfeda0000-0xfedbffff.  This affects both the "non-SMM" view controlled
> by D_OPEN and the SMM view controlled by G_SMRAME, so add two new
> MemoryRegions and toggle the enabled/disabled state of all four
> in mch_update_smram.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Acked-by: Michael S. Tsirkin <address@hidden>

> ---
>  hw/pci-host/q35.c         | 35 +++++++++++++++++++++++++++++++----
>  include/hw/pci-host/q35.h | 16 ++++++++--------
>  2 files changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 236d3f5..92804fe 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -266,12 +266,29 @@ static void mch_update_pam(MCHPCIState *mch)
>  static void mch_update_smram(MCHPCIState *mch)
>  {
>      PCIDevice *pd = PCI_DEVICE(mch);
> +    bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & 
> MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME);
>  
>      memory_region_transaction_begin();
> -    memory_region_set_enabled(&mch->smram_region,
> -                              !(pd->config[MCH_HOST_BRIDGE_SMRAM] & 
> SMRAM_D_OPEN));
> -    memory_region_set_enabled(&mch->smram,
> -                              pd->config[MCH_HOST_BRIDGE_SMRAM] & 
> SMRAM_G_SMRAME);
> +
> +    if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) {
> +        /* Hide (!) low SMRAM if H_SMRAME = 1 */
> +        memory_region_set_enabled(&mch->smram_region, h_smrame);
> +        /* Show high SMRAM if H_SMRAME = 1 */
> +        memory_region_set_enabled(&mch->open_high_smram, h_smrame);
> +    } else {
> +        /* Hide high SMRAM and low SMRAM */
> +        memory_region_set_enabled(&mch->smram_region, true);
> +        memory_region_set_enabled(&mch->open_high_smram, false);
> +    }
> +
> +    if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_G_SMRAME) {
> +        memory_region_set_enabled(&mch->low_smram, !h_smrame);
> +        memory_region_set_enabled(&mch->high_smram, h_smrame);
> +    } else {
> +        memory_region_set_enabled(&mch->low_smram, false);
> +        memory_region_set_enabled(&mch->high_smram, false);
> +    }
> +
>      memory_region_transaction_commit();
>  }
>  
> @@ -400,6 +417,12 @@ static void mch_realize(PCIDevice *d, Error **errp)
>                                          &mch->smram_region, 1);
>      memory_region_set_enabled(&mch->smram_region, true);
>  
> +    memory_region_init_alias(&mch->open_high_smram, OBJECT(mch), 
> "smram-open-high",
> +                             mch->ram_memory, 0xa0000, 0x20000);
> +    memory_region_add_subregion_overlap(mch->system_memory, 0xfeda0000,
> +                                        &mch->open_high_smram, 1);
> +    memory_region_set_enabled(&mch->open_high_smram, false);
> +
>      /* smram, as seen by SMM CPUs */
>      memory_region_init(&mch->smram, OBJECT(mch), "smram", 1ull << 32);
>      memory_region_set_enabled(&mch->smram, true);
> @@ -407,6 +430,10 @@ static void mch_realize(PCIDevice *d, Error **errp)
>                               mch->ram_memory, 0xa0000, 0x20000);
>      memory_region_set_enabled(&mch->low_smram, true);
>      memory_region_add_subregion(&mch->smram, 0xa0000, &mch->low_smram);
> +    memory_region_init_alias(&mch->high_smram, OBJECT(mch), "smram-high",
> +                             mch->ram_memory, 0xa0000, 0x20000);
> +    memory_region_set_enabled(&mch->high_smram, true);
> +    memory_region_add_subregion(&mch->smram, 0xfeda0000, &mch->high_smram);
>      object_property_add_const_link(qdev_get_machine(), "smram",
>                                  OBJECT(&mch->smram), &error_abort);
>  
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index 17adeaa..0fff6a2 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -52,8 +52,8 @@ typedef struct MCHPCIState {
>      MemoryRegion *system_memory;
>      MemoryRegion *address_space_io;
>      PAMMemoryRegion pam_regions[13];
> -    MemoryRegion smram_region;
> -    MemoryRegion smram, low_smram;
> +    MemoryRegion smram_region, open_high_smram;
> +    MemoryRegion smram, low_smram, high_smram;
>      PcPciInfo pci_info;
>      ram_addr_t below_4g_mem_size;
>      ram_addr_t above_4g_mem_size;
> @@ -127,7 +127,7 @@ typedef struct Q35PCIHost {
>  #define MCH_HOST_BRIDGE_PAM_MASK               ((uint8_t)0x3)
>  
>  #define MCH_HOST_BRIDGE_SMRAM                  0x9d
> -#define MCH_HOST_BRIDGE_SMRAM_SIZE             1
> +#define MCH_HOST_BRIDGE_SMRAM_SIZE             2
>  #define MCH_HOST_BRIDGE_SMRAM_DEFAULT          ((uint8_t)0x2)
>  #define MCH_HOST_BRIDGE_SMRAM_D_OPEN           ((uint8_t)(1 << 6))
>  #define MCH_HOST_BRIDGE_SMRAM_D_CLS            ((uint8_t)(1 << 5))
> @@ -141,11 +141,11 @@ typedef struct Q35PCIHost {
>  #define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END  0x100000
>  
>  #define MCH_HOST_BRIDGE_ESMRAMC                0x9e
> -#define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME       ((uint8_t)(1 << 6))
> -#define MCH_HOST_BRIDGE_ESMRAMC_E_SMERR        ((uint8_t)(1 << 5))
> -#define MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE       ((uint8_t)(1 << 4))
> -#define MCH_HOST_BRIDGE_ESMRAMC_SM_L1          ((uint8_t)(1 << 3))
> -#define MCH_HOST_BRIDGE_ESMRAMC_SM_L2          ((uint8_t)(1 << 2))
> +#define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME       ((uint8_t)(1 << 7))
> +#define MCH_HOST_BRIDGE_ESMRAMC_E_SMERR        ((uint8_t)(1 << 6))
> +#define MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE       ((uint8_t)(1 << 5))
> +#define MCH_HOST_BRIDGE_ESMRAMC_SM_L1          ((uint8_t)(1 << 4))
> +#define MCH_HOST_BRIDGE_ESMRAMC_SM_L2          ((uint8_t)(1 << 3))
>  #define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK   ((uint8_t)(0x3 << 1))
>  #define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_1MB    ((uint8_t)(0x0 << 1))
>  #define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_2MB    ((uint8_t)(0x1 << 1))
> -- 
> 2.4.1
>