From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YrpSb-0006Fn-Fc for qemu-devel@nongnu.org; Mon, 11 May 2015 11:17:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YrpSY-0008DX-Ls for qemu-devel@nongnu.org; Mon, 11 May 2015 11:17:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58355) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YrpSY-0008Cf-F8 for qemu-devel@nongnu.org; Mon, 11 May 2015 11:17:34 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4BFHXhs001389 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 11 May 2015 11:17:33 -0400 Message-ID: <5550C808.8000505@redhat.com> Date: Mon, 11 May 2015 17:17:28 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1431352157-40283-1-git-send-email-pbonzini@redhat.com> <1431352157-40283-24-git-send-email-pbonzini@redhat.com> In-Reply-To: <1431352157-40283-24-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 23/31] ich9: implement SMI_LOCK List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , kraxel@redhat.com Cc: qemu-devel@nongnu.org, mst@redhat.com On 05/11/15 15:49, Paolo Bonzini wrote: > From: Gerd Hoffmann > > Signed-off-by: Gerd Hoffmann > Signed-off-by: Paolo Bonzini > --- > hw/acpi/ich9.c | 4 +++- > hw/isa/lpc_ich9.c | 19 +++++++++++++++++++ > include/hw/acpi/ich9.h | 1 + > include/hw/i386/ich9.h | 6 ++++++ > 4 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 5352e19..310aa64 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -94,7 +94,8 @@ static void ich9_smi_writel(void *opaque, hwaddr addr, uint64_t val, > ICH9LPCPMRegs *pm = opaque; > switch (addr) { > case 0: > - pm->smi_en = val; > + pm->smi_en &= ~pm->smi_en_wmask; > + pm->smi_en |= (val & pm->smi_en_wmask); > break; > } > } > @@ -198,6 +199,7 @@ static void pm_reset(void *opaque) > * support SMM mode. */ > pm->smi_en |= ICH9_PMIO_SMI_EN_APMC_EN; > } > + pm->smi_en_wmask = ~0; > > acpi_update_sci(&pm->acpi_regs, pm->irq); > } > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index dba7585..0269cfe 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -410,12 +410,28 @@ static void ich9_lpc_rcba_update(ICH9LPCState *lpc, uint32_t rbca_old) > } > } > > +/* config:GEN_PMCON* */ > +static void > +ich9_lpc_pmcon_update(ICH9LPCState *lpc) > +{ > + uint16_t gen_pmcon_1 = pci_get_word(lpc->d.config + ICH9_LPC_GEN_PMCON_1); > + uint16_t wmask; > + > + if (gen_pmcon_1 & ICH9_LPC_GEN_PMCON_1_SMI_LOCK) { > + wmask = pci_get_word(lpc->d.wmask + ICH9_LPC_GEN_PMCON_1); > + wmask &= ~ICH9_LPC_GEN_PMCON_1_SMI_LOCK; > + pci_set_word(lpc->d.wmask + ICH9_LPC_GEN_PMCON_1, wmask); > + lpc->pm.smi_en_wmask &= ~1; If I understand correctly, this makes SMI_LOCK lock down the GBL_SMI_EN bit (and my OVMF patch that relies on that / tests it is satisfied too). But, it doesn't seem to lock down APMC_EN. According to the ICH9 spec, it doesn't need to -- however when we discussed this earlier (see Message-Id: <553F4D23.3060305@redhat.com>), the idea was to lock down APMC_EN as well. (And I don't understand why the ICH9 spec / hw implementation doesn't lock APMC_EN; without that, APM_CNT won't necessarily trigger an SMI.) Thanks! Laszlo > + } > +} > + > static int ich9_lpc_post_load(void *opaque, int version_id) > { > ICH9LPCState *lpc = opaque; > > ich9_lpc_pmbase_update(lpc); > ich9_lpc_rcba_update(lpc, 0 /* disabled ICH9_LPC_RBCA_EN */); > + ich9_lpc_pmcon_update(lpc); > return 0; > } > > @@ -438,6 +454,9 @@ static void ich9_lpc_config_write(PCIDevice *d, > if (ranges_overlap(addr, len, ICH9_LPC_PIRQE_ROUT, 4)) { > pci_bus_fire_intx_routing_notifier(lpc->d.bus); > } > + if (ranges_overlap(addr, len, ICH9_LPC_GEN_PMCON_1, 8)) { > + ich9_lpc_pmcon_update(lpc); > + } > } > > static void ich9_lpc_reset(DeviceState *qdev) > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > index c2d3dba..77cc65c 100644 > --- a/include/hw/acpi/ich9.h > +++ b/include/hw/acpi/ich9.h > @@ -39,6 +39,7 @@ typedef struct ICH9LPCPMRegs { > MemoryRegion io_smi; > > uint32_t smi_en; > + uint32_t smi_en_wmask; > uint32_t smi_sts; > > qemu_irq irq; /* SCI */ > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h > index f4e522c..a2cc15c 100644 > --- a/include/hw/i386/ich9.h > +++ b/include/hw/i386/ich9.h > @@ -152,6 +152,12 @@ Object *ich9_lpc_find(void); > #define ICH9_LPC_PIRQ_ROUT_MASK Q35_MASK(8, 3, 0) > #define ICH9_LPC_PIRQ_ROUT_DEFAULT 0x80 > > +#define ICH9_LPC_GEN_PMCON_1 0xa0 > +#define ICH9_LPC_GEN_PMCON_1_SMI_LOCK (1 << 4) > +#define ICH9_LPC_GEN_PMCON_2 0xa2 > +#define ICH9_LPC_GEN_PMCON_3 0xa4 > +#define ICH9_LPC_GEN_PMCON_LOCK 0xa6 > + > #define ICH9_LPC_RCBA 0xf0 > #define ICH9_LPC_RCBA_BA_MASK Q35_MASK(32, 31, 14) > #define ICH9_LPC_RCBA_EN 0x1 > Thanks Laszlo