All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled
@ 2017-05-16  1:21 Anthony Xu
  2017-05-16 14:17 ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Anthony Xu @ 2017-05-16  1:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Anthony Xu

when smm is disabled, smram is not used, so disable it

Signed-off-by: Anthony Xu <anthony.xu@intel.com>
---
 hw/pci-host/piix.c | 45 +++++++++++++++--------------
 hw/pci-host/q35.c  | 83 +++++++++++++++++++++++++++++-------------------------
 kvm-all.c          |  3 +-
 target/i386/kvm.c  |  2 +-
 4 files changed, 70 insertions(+), 63 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index bf4221d..ce43f87 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -142,10 +142,12 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
         pam_update(&d->pam_regions[i], i,
                    pd->config[I440FX_PAM + ((i + 1) / 2)]);
     }
-    memory_region_set_enabled(&d->smram_region,
-                              !(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
-    memory_region_set_enabled(&d->smram,
-                              pd->config[I440FX_SMRAM] & SMRAM_G_SMRAME);
+    if (pc_machine_is_smm_enabled(PC_MACHINE(current_machine))) {
+        memory_region_set_enabled(&d->smram_region,
+                !(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
+        memory_region_set_enabled(&d->smram,
+                pd->config[I440FX_SMRAM] & SMRAM_G_SMRAME);
+    }
     memory_region_transaction_commit();
 }
 
@@ -355,23 +357,24 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
     pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
                            f->pci_address_space);
 
-    /* if *disabled* show SMRAM to all CPUs */
-    memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
-                             f->pci_address_space, 0xa0000, 0x20000);
-    memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
-                                        &f->smram_region, 1);
-    memory_region_set_enabled(&f->smram_region, true);
-
-    /* smram, as seen by SMM CPUs */
-    memory_region_init(&f->smram, OBJECT(d), "smram", 1ull << 32);
-    memory_region_set_enabled(&f->smram, true);
-    memory_region_init_alias(&f->low_smram, OBJECT(d), "smram-low",
-                             f->ram_memory, 0xa0000, 0x20000);
-    memory_region_set_enabled(&f->low_smram, true);
-    memory_region_add_subregion(&f->smram, 0xa0000, &f->low_smram);
-    object_property_add_const_link(qdev_get_machine(), "smram",
-                                   OBJECT(&f->smram), &error_abort);
-
+    if (pc_machine_is_smm_enabled(PC_MACHINE(current_machine))) {
+        /* if *disabled* show SMRAM to all CPUs */
+        memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
+                f->pci_address_space, 0xa0000, 0x20000);
+        memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
+                &f->smram_region, 1);
+        memory_region_set_enabled(&f->smram_region, true);
+
+        /* smram, as seen by SMM CPUs */
+        memory_region_init(&f->smram, OBJECT(d), "smram", 1ull << 32);
+        memory_region_set_enabled(&f->smram, true);
+        memory_region_init_alias(&f->low_smram, OBJECT(d), "smram-low",
+                f->ram_memory, 0xa0000, 0x20000);
+        memory_region_set_enabled(&f->low_smram, true);
+        memory_region_add_subregion(&f->smram, 0xa0000, &f->low_smram);
+        object_property_add_const_link(qdev_get_machine(), "smram",
+                OBJECT(&f->smram), &error_abort);
+    }
     init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
              &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
     for (i = 0; i < 12; ++i) {
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b..a10d79e 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -324,6 +324,9 @@ static void mch_update_pam(MCHPCIState *mch)
 /* SMRAM */
 static void mch_update_smram(MCHPCIState *mch)
 {
+    if (!pc_machine_is_smm_enabled(PC_MACHINE(current_machine))) {
+        return;
+    }
     PCIDevice *pd = PCI_DEVICE(mch);
     bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME);
     uint32_t tseg_size;
@@ -469,46 +472,48 @@ static void mch_realize(PCIDevice *d, Error **errp)
     pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
                            mch->pci_address_space);
 
-    /* if *disabled* show SMRAM to all CPUs */
-    memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
-                             mch->pci_address_space, 0xa0000, 0x20000);
-    memory_region_add_subregion_overlap(mch->system_memory, 0xa0000,
-                                        &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);
-    memory_region_init_alias(&mch->low_smram, OBJECT(mch), "smram-low",
-                             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);
-
-    memory_region_init_io(&mch->tseg_blackhole, OBJECT(mch),
-                          &tseg_blackhole_ops, NULL,
-                          "tseg-blackhole", 0);
-    memory_region_set_enabled(&mch->tseg_blackhole, false);
-    memory_region_add_subregion_overlap(mch->system_memory,
-                                        mch->below_4g_mem_size,
-                                        &mch->tseg_blackhole, 1);
+    if (pc_machine_is_smm_enabled(PC_MACHINE(current_machine))) {
+        /* if *disabled* show SMRAM to all CPUs */
+        memory_region_init_alias(&mch->smram_region, OBJECT(mch),
+               "smram-region", mch->pci_address_space, 0xa0000, 0x20000);
+        memory_region_add_subregion_overlap(mch->system_memory, 0xa0000,
+                &mch->smram_region, 1);
+        memory_region_set_enabled(&mch->smram_region, true);
 
-    memory_region_init_alias(&mch->tseg_window, OBJECT(mch), "tseg-window",
-                             mch->ram_memory, mch->below_4g_mem_size, 0);
-    memory_region_set_enabled(&mch->tseg_window, false);
-    memory_region_add_subregion(&mch->smram, mch->below_4g_mem_size,
-                                &mch->tseg_window);
-    object_property_add_const_link(qdev_get_machine(), "smram",
-                                   OBJECT(&mch->smram), &error_abort);
+        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);
+        memory_region_init_alias(&mch->low_smram, OBJECT(mch), "smram-low",
+                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);
+
+        memory_region_init_io(&mch->tseg_blackhole, OBJECT(mch),
+                &tseg_blackhole_ops, NULL,
+                "tseg-blackhole", 0);
+        memory_region_set_enabled(&mch->tseg_blackhole, false);
+        memory_region_add_subregion_overlap(mch->system_memory,
+                mch->below_4g_mem_size,
+                &mch->tseg_blackhole, 1);
+
+        memory_region_init_alias(&mch->tseg_window, OBJECT(mch), "tseg-window",
+                mch->ram_memory, mch->below_4g_mem_size, 0);
+        memory_region_set_enabled(&mch->tseg_window, false);
+        memory_region_add_subregion(&mch->smram, mch->below_4g_mem_size,
+                &mch->tseg_window);
+        object_property_add_const_link(qdev_get_machine(), "smram",
+                OBJECT(&mch->smram), &error_abort);
+    }
 
     init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
              mch->pci_address_space, &mch->pam_regions[0],
diff --git a/kvm-all.c b/kvm-all.c
index 90b8573..1250fff 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1583,6 +1583,7 @@ static int kvm_init(MachineState *ms)
     const char *kvm_type;
 
     s = KVM_STATE(ms->accelerator);
+    kvm_state = s;
 
     /*
      * On systems where the kernel can support different base page
@@ -1755,8 +1756,6 @@ static int kvm_init(MachineState *ms)
         kvm_irqchip_create(ms, s);
     }
 
-    kvm_state = s;
-
     if (kvm_eventfds_allowed) {
         s->memory_listener.listener.eventfd_add = kvm_mem_ioeventfd_add;
         s->memory_listener.listener.eventfd_del = kvm_mem_ioeventfd_del;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 55865db..65716b6 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1254,7 +1254,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         }
     }
 
-    if (kvm_check_extension(s, KVM_CAP_X86_SMM)) {
+    if (pc_machine_is_smm_enabled(PC_MACHINE(ms))) {
         smram_machine_done.notify = register_smram_listener;
         qemu_add_machine_init_done_notifier(&smram_machine_done);
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled
  2017-05-16  1:21 [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled Anthony Xu
@ 2017-05-16 14:17 ` Paolo Bonzini
  2017-05-16 20:22   ` Xu, Anthony
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2017-05-16 14:17 UTC (permalink / raw)
  To: Anthony Xu, qemu-devel



On 16/05/2017 03:21, Anthony Xu wrote:
> when smm is disabled, smram is not used, so disable it
> 
> Signed-off-by: Anthony Xu <anthony.xu@intel.com>

What is the benefit?

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled
  2017-05-16 14:17 ` Paolo Bonzini
@ 2017-05-16 20:22   ` Xu, Anthony
  2017-05-18 22:00     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Xu, Anthony @ 2017-05-16 20:22 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

> On 16/05/2017 03:21, Anthony Xu wrote:
> > when smm is disabled, smram is not used, so disable it
> >
> > Signed-off-by: Anthony Xu <anthony.xu@intel.com>
> 
> What is the benefit?
This patch removes 1 memory region for i440 platform and 3 memory regions
for q35 platform. That makes functions which iterates memory region tree
a little bit fast even the memory regions are disabled.


Anthony

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled
  2017-05-16 20:22   ` Xu, Anthony
@ 2017-05-18 22:00     ` Paolo Bonzini
  2017-05-18 23:33       ` Xu, Anthony
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2017-05-18 22:00 UTC (permalink / raw)
  To: Xu, Anthony, qemu-devel



On 16/05/2017 22:22, Xu, Anthony wrote:
>> On 16/05/2017 03:21, Anthony Xu wrote:
>>> when smm is disabled, smram is not used, so disable it
>>>
>>> Signed-off-by: Anthony Xu <anthony.xu@intel.com>
>>
>> What is the benefit?
> 
> This patch removes 1 memory region for i440 platform and 3 memory regions
> for q35 platform. That makes functions which iterates memory region tree
> a little bit fast even the memory regions are disabled.

Does it translate to anything measurable in benchmarks?  Could you leave
the regions there, but skip the creation of the SMRAM address space in
register_smram_listener when the machine doesn't have SMM enabled?

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled
  2017-05-18 22:00     ` Paolo Bonzini
@ 2017-05-18 23:33       ` Xu, Anthony
  0 siblings, 0 replies; 5+ messages in thread
From: Xu, Anthony @ 2017-05-18 23:33 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

> On 16/05/2017 22:22, Xu, Anthony wrote:
> >> On 16/05/2017 03:21, Anthony Xu wrote:
> >>> when smm is disabled, smram is not used, so disable it
> >>>
> >>> Signed-off-by: Anthony Xu <anthony.xu@intel.com>
> >>
> >> What is the benefit?
> >
> > This patch removes 1 memory region for i440 platform and 3 memory
> regions
> > for q35 platform. That makes functions which iterates memory region tree
> > a little bit fast even the memory regions are disabled.
> 
> Does it translate to anything measurable in benchmarks?  
Yes , we see boot time improvement with this patch in our setup (skip guest BIOS, 
disable guest PAM).

>Could you leave
> the regions there, but skip the creation of the SMRAM address space in
> register_smram_listener when the machine doesn't have SMM enabled?
Sounds like you have concerns on removing smram regions.
What are your concerns?

-Anthony

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-05-18 23:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16  1:21 [Qemu-devel] [PATCH] SMM: disable smram region if smm is disabled Anthony Xu
2017-05-16 14:17 ` Paolo Bonzini
2017-05-16 20:22   ` Xu, Anthony
2017-05-18 22:00     ` Paolo Bonzini
2017-05-18 23:33       ` Xu, Anthony

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.