Re: [RFC 1/1] hw: tpmtisspi: add SPI support to QEMU TPM implementation
Hi Guenter,
Not from me at this moment :)
Thanks,
Iris
On Wed, Dec 13, 2023 at 9:39 AM Guenter Roeck wrote:
> Hi,
>
> On Tue, Aug 02, 2022 at 07:32:41PM -0700, Iris Chen wrote:
> > From: Iris Chen
> >
> > Signed-off-by: Iris Chen
> > ---
>
> Are there any plans to submit a new version of this patch ?
>
> Thanks,
> Guenter
>
> > configs/devices/arm-softmmu/default.mak | 1 +
> > hw/arm/Kconfig | 5 +
> > hw/tpm/Kconfig | 5 +
> > hw/tpm/meson.build | 1 +
> > hw/tpm/tpm_tis_spi.c| 311
> > include/sysemu/tpm.h| 3 +
> > 6 files changed, 326 insertions(+)
> > create mode 100644 hw/tpm/tpm_tis_spi.c
> >
> > diff --git a/configs/devices/arm-softmmu/default.mak
> b/configs/devices/arm-softmmu/default.mak
> > index 6985a25377..80d2841568 100644
> > --- a/configs/devices/arm-softmmu/default.mak
> > +++ b/configs/devices/arm-softmmu/default.mak
> > @@ -42,3 +42,4 @@ CONFIG_FSL_IMX6UL=y
> > CONFIG_SEMIHOSTING=y
> > CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> > CONFIG_ALLWINNER_H3=y
> > +CONFIG_FBOBMC_AST=y
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 15fa79afd3..193decaec1 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -458,6 +458,11 @@ config ASPEED_SOC
> > select PMBUS
> > select MAX31785
> >
> > +config FBOBMC_AST
> > +bool
> > +select ASPEED_SOC
> > +select TPM_TIS_SPI
> > +
> > config MPS2
> > bool
> > imply I2C_DEVICES
> > diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
> > index 29e82f3c92..370a43f045 100644
> > --- a/hw/tpm/Kconfig
> > +++ b/hw/tpm/Kconfig
> > @@ -8,6 +8,11 @@ config TPM_TIS_SYSBUS
> > depends on TPM
> > select TPM_TIS
> >
> > +config TPM_TIS_SPI
> > +bool
> > +depends on TPM
> > +select TPM_TIS
> > +
> > config TPM_TIS
> > bool
> > depends on TPM
> > diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> > index 1c68d81d6a..1a057f4e36 100644
> > --- a/hw/tpm/meson.build
> > +++ b/hw/tpm/meson.build
> > @@ -2,6 +2,7 @@ softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true:
> files('tpm_tis_common.c'))
> > softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true:
> files('tpm_tis_isa.c'))
> > softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true:
> files('tpm_tis_sysbus.c'))
> > softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
> > +softmmu_ss.add(when: 'CONFIG_TPM_TIS_SPI', if_true:
> files('tpm_tis_spi.c'))
> >
> > specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true:
> files('tpm_ppi.c'))
> > specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true:
> files('tpm_ppi.c'))
> > diff --git a/hw/tpm/tpm_tis_spi.c b/hw/tpm/tpm_tis_spi.c
> > new file mode 100644
> > index 00..c98ddcfddb
> > --- /dev/null
> > +++ b/hw/tpm/tpm_tis_spi.c
> > @@ -0,0 +1,311 @@
> > +#include "qemu/osdep.h"
> > +#include "hw/qdev-properties.h"
> > +#include "migration/vmstate.h"
> > +#include "hw/acpi/tpm.h"
> > +#include "tpm_prop.h"
> > +#include "tpm_tis.h"
> > +#include "qom/object.h"
> > +#include "hw/ssi/ssi.h"
> > +#include "hw/ssi/spi_gpio.h"
> > +
> > +#define TPM_TIS_SPI_ADDR_BYTES 3
> > +#define SPI_WRITE 0
> > +
> > +typedef enum {
> > +TIS_SPI_PKT_STATE_DEACTIVATED = 0,
> > +TIS_SPI_PKT_STATE_START,
> > +TIS_SPI_PKT_STATE_ADDRESS,
> > +TIS_SPI_PKT_STATE_DATA_WR,
> > +TIS_SPI_PKT_STATE_DATA_RD,
> > +TIS_SPI_PKT_STATE_DONE,
> > +} TpmTisSpiPktState;
> > +
> > +union TpmTisRWSizeByte {
> > +uint8_t byte;
> > +struct {
> > +uint8_t data_expected_size:6;
> > +uint8_t resv:1;
> > +uint8_t rwflag:1;
> > +};
> > +};
> > +
> > +union TpmTisSpiHwAddr {
> > +hwaddr addr;
> > +uint8_t bytes[sizeof(hwaddr)];
> > +};
> > +
> > +union TpmTisSpiData {
> > +uint32_t data;
> > +uint8_t bytes[64];
> > +};
> > +
> > +struct TpmTisSpiState {
> > +/*< private >*/
> > +SSIPeripheral parent_obj;
> > +
> > +/*< public >*/
> > +TPMState tpm_state; /* not a QOM object */
> >
[RFC 0/1] SPI support in QEMU TPM
From: Iris Chen Hey everyone, Thanks for all your comments on the SPI GPIO model. I am working through them. As for adding support for SPI-based TPMs in QEMU, this RFC patch adds SPI support in the QEMU TPM implementation via tpm_tis_spi.c. The QEMU tree already has support for two connection methods to the TPM: mmio (isa for x86, sysbus for arm) and “spapr”. This patch adds a new SPI bus implementation for the TPM. Specifically, this SPI bus implementation connects to the Yosemite-v3.5 model using the SPI-GPIO model sent earlier last week. I have already tested these implementations locally together and can verify that the Linux kernel can successfully probe the TPM device on Yosemite-v3.5 and we can run the TPM command line tools to interact with it. Please let me know what you think about this! Thanks, Iris Iris Chen (1): hw: tpmtisspi: add SPI support to QEMU TPM implementation configs/devices/arm-softmmu/default.mak | 1 + hw/arm/Kconfig | 5 + hw/tpm/Kconfig | 5 + hw/tpm/meson.build | 1 + hw/tpm/tpm_tis_spi.c| 311 include/sysemu/tpm.h| 3 + 6 files changed, 326 insertions(+) create mode 100644 hw/tpm/tpm_tis_spi.c -- 2.30.2
[RFC 1/1] hw: tpmtisspi: add SPI support to QEMU TPM implementation
From: Iris Chen
Signed-off-by: Iris Chen
---
configs/devices/arm-softmmu/default.mak | 1 +
hw/arm/Kconfig | 5 +
hw/tpm/Kconfig | 5 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_spi.c| 311
include/sysemu/tpm.h| 3 +
6 files changed, 326 insertions(+)
create mode 100644 hw/tpm/tpm_tis_spi.c
diff --git a/configs/devices/arm-softmmu/default.mak
b/configs/devices/arm-softmmu/default.mak
index 6985a25377..80d2841568 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -42,3 +42,4 @@ CONFIG_FSL_IMX6UL=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
CONFIG_ALLWINNER_H3=y
+CONFIG_FBOBMC_AST=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 15fa79afd3..193decaec1 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -458,6 +458,11 @@ config ASPEED_SOC
select PMBUS
select MAX31785
+config FBOBMC_AST
+bool
+select ASPEED_SOC
+select TPM_TIS_SPI
+
config MPS2
bool
imply I2C_DEVICES
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..370a43f045 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -8,6 +8,11 @@ config TPM_TIS_SYSBUS
depends on TPM
select TPM_TIS
+config TPM_TIS_SPI
+bool
+depends on TPM
+select TPM_TIS
+
config TPM_TIS
bool
depends on TPM
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 1c68d81d6a..1a057f4e36 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/meson.build
@@ -2,6 +2,7 @@ softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true:
files('tpm_tis_common.c'))
softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true: files('tpm_tis_isa.c'))
softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true:
files('tpm_tis_sysbus.c'))
softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
+softmmu_ss.add(when: 'CONFIG_TPM_TIS_SPI', if_true: files('tpm_tis_spi.c'))
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true:
files('tpm_ppi.c'))
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true:
files('tpm_ppi.c'))
diff --git a/hw/tpm/tpm_tis_spi.c b/hw/tpm/tpm_tis_spi.c
new file mode 100644
index 00..c98ddcfddb
--- /dev/null
+++ b/hw/tpm/tpm_tis_spi.c
@@ -0,0 +1,311 @@
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "hw/acpi/tpm.h"
+#include "tpm_prop.h"
+#include "tpm_tis.h"
+#include "qom/object.h"
+#include "hw/ssi/ssi.h"
+#include "hw/ssi/spi_gpio.h"
+
+#define TPM_TIS_SPI_ADDR_BYTES 3
+#define SPI_WRITE 0
+
+typedef enum {
+TIS_SPI_PKT_STATE_DEACTIVATED = 0,
+TIS_SPI_PKT_STATE_START,
+TIS_SPI_PKT_STATE_ADDRESS,
+TIS_SPI_PKT_STATE_DATA_WR,
+TIS_SPI_PKT_STATE_DATA_RD,
+TIS_SPI_PKT_STATE_DONE,
+} TpmTisSpiPktState;
+
+union TpmTisRWSizeByte {
+uint8_t byte;
+struct {
+uint8_t data_expected_size:6;
+uint8_t resv:1;
+uint8_t rwflag:1;
+};
+};
+
+union TpmTisSpiHwAddr {
+hwaddr addr;
+uint8_t bytes[sizeof(hwaddr)];
+};
+
+union TpmTisSpiData {
+uint32_t data;
+uint8_t bytes[64];
+};
+
+struct TpmTisSpiState {
+/*< private >*/
+SSIPeripheral parent_obj;
+
+/*< public >*/
+TPMState tpm_state; /* not a QOM object */
+TpmTisSpiPktState tpm_tis_spi_state;
+
+union TpmTisRWSizeByte first_byte;
+union TpmTisSpiHwAddr addr;
+union TpmTisSpiData data;
+
+uint32_t data_size;
+uint8_t data_idx;
+uint8_t addr_idx;
+};
+
+struct TpmTisSpiClass {
+SSIPeripheralClass parent_class;
+};
+
+OBJECT_DECLARE_TYPE(TpmTisSpiState, TpmTisSpiClass, TPM_TIS_SPI)
+
+static void tpm_tis_spi_mmio_read(TpmTisSpiState *tts)
+{
+uint16_t offset = tts->addr.addr & 0xffc;
+
+switch (offset) {
+case TPM_TIS_REG_DATA_FIFO:
+for (uint8_t i = 0; i < tts->data_size; i++) {
+tts->data.bytes[i] = (uint8_t)tpm_tis_memory_ops.read(
+ >tpm_state,
+ tts->addr.addr,
+ 1);
+}
+break;
+default:
+tts->data.data = (uint32_t)tpm_tis_memory_ops.read(
+ >tpm_state,
+ tts->addr.addr,
+ tts->data_size);
+}
+}
+
+static void tpm_tis_spi_mmio_write(TpmTisSpiState *tts)
+{
+uint16_t offset = tts->addr.addr & 0xffc;
+
+switch (offset) {
+case TPM_TIS_REG_DATA_FIFO:
+for (uint8_t i = 0; i < tts->data_size; i++) {
+tpm_tis_memory_ops.write(>tpm_state,
+ tts->addr.addr,
+ tts->data.bytes[i],
+
Re: [RFC 0/3] Add Generic SPI GPIO model
Thanks everyone for the insightful feedback! This is really helpful for me. I am taking a look at all the comments now and will investigate into it. Best, Iris
[RFC 2/3] hw: spi_gpio: add spi gpio model
Signed-off-by: Iris Chen
---
hw/ssi/spi_gpio.c | 166 ++
include/hw/ssi/spi_gpio.h | 38 +
2 files changed, 204 insertions(+)
create mode 100644 hw/ssi/spi_gpio.c
create mode 100644 include/hw/ssi/spi_gpio.h
diff --git a/hw/ssi/spi_gpio.c b/hw/ssi/spi_gpio.c
new file mode 100644
index 00..1e99c55933
--- /dev/null
+++ b/hw/ssi/spi_gpio.c
@@ -0,0 +1,166 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * This code is licensed under the GPL version 2 or later. See the COPYING
+ * file in the top-level directory.
+ */
+
+#include "hw/ssi/spi_gpio.h"
+#include "hw/irq.h"
+
+#define SPI_CPHA BIT(0) /* clock phase (1 = SPI_CLOCK_PHASE_SECOND) */
+#define SPI_CPOL BIT(1) /* clock polarity (1 = SPI_POLARITY_HIGH) */
+
+static void do_leading_edge(SpiGpioState *s)
+{
+if (!s->CPHA) {
+s->input_bits |= object_property_get_bool(OBJECT(s->controller_state),
+ "gpioX4", NULL);
+/*
+ * According to SPI protocol:
+ * CPHA=0 leading half clock cycle is sampling phase
+ * We technically should not drive out miso
+ * However, when the kernel bitbang driver is setting the clk pin,
+ * it will overwrite the miso value, so we are driving out miso in
+ * the sampling half clock cycle as well to workaround this issue
+ */
+s->miso = !!(s->output_bits & 0x80);
+object_property_set_bool(OBJECT(s->controller_state), "gpioX5",
s->miso,
+ NULL);
+}
+}
+
+static void do_trailing_edge(SpiGpioState *s)
+{
+if (s->CPHA) {
+s->input_bits |= object_property_get_bool(OBJECT(s->controller_state),
+ "gpioX4", NULL);
+/*
+ * According to SPI protocol:
+ * CPHA=1 trailing half clock cycle is sampling phase
+ * We technically should not drive out miso
+ * However, when the kernel bitbang driver is setting the clk pin,
+ * it will overwrite the miso value, so we are driving out miso in
+ * the sampling half clock cycle as well to workaround this issue
+ */
+s->miso = !!(s->output_bits & 0x80);
+object_property_set_bool(OBJECT(s->controller_state), "gpioX5",
s->miso,
+ NULL);
+}
+}
+
+static void cs_set_level(void *opaque, int n, int level)
+{
+SpiGpioState *s = SPI_GPIO(opaque);
+s->cs = !!level;
+
+/* relay the CS value to the CS output pin */
+qemu_set_irq(s->cs_output_pin, s->cs);
+
+s->miso = !!(s->output_bits & 0x80);
+object_property_set_bool(OBJECT(s->controller_state),
+ "gpioX5", s->miso, NULL);
+
+s->clk = !!(s->mode & SPI_CPOL);
+}
+
+static void clk_set_level(void *opaque, int n, int level)
+{
+SpiGpioState *s = SPI_GPIO(opaque);
+
+bool cur = !!level;
+
+/* CS# is high/not selected, do nothing */
+if (s->cs) {
+return;
+}
+
+/* When the lock has not changed, do nothing */
+if (s->clk == cur) {
+return;
+}
+
+s->clk = cur;
+
+/* Leading edge */
+if (s->clk != s->CIDLE) {
+do_leading_edge(s);
+}
+
+/* Trailing edge */
+if (s->clk == s->CIDLE) {
+do_trailing_edge(s);
+s->clk_counter++;
+
+/*
+ * Deliver the input to and
+ * get the next output byte
+ * from the SPI device
+ */
+if (s->clk_counter == 8) {
+s->output_bits = ssi_transfer(s->spi, s->input_bits);
+s->clk_counter = 0;
+s->input_bits = 0;
+ } else {
+s->input_bits <<= 1;
+s->output_bits <<= 1;
+ }
+}
+}
+
+static void spi_gpio_realize(DeviceState *dev, Error **errp)
+{
+SpiGpioState *s = SPI_GPIO(dev);
+
+s->spi = ssi_create_bus(dev, "spi");
+s->spi->preread = true;
+
+s->mode = 0;
+s->clk_counter = 0;
+
+s->cs = true;
+s->clk = true;
+
+/* Assuming the first output byte is 0 */
+s->output_bits = 0;
+s->CIDLE = !!(s->mode & SPI_CPOL);
+s->CPHA = !!(s->mode & SPI_CPHA);
+
+/* init the input GPIO lines */
+/* SPI_CS_in connects to the Aspeed GPIO */
+qdev_init_gpio_in_named(dev, cs_set_level, "SPI_CS_in", 1);
+qdev_init_gpio_in_named(dev, clk_set_level, "SPI_CLK", 1);
+
+/* init the output GPIO lines */
+/* SPI_CS_out connects to the SSI_GPIO_CS */
+qdev_init_gpio_out_named(dev, >cs_output_pin, "SPI_CS_out", 1);
+
+qdev_connect_gpio_out_named(s->controller_
[RFC 1/3] hw: m25p80: add prereading ability in transfer8
With SPI-GPIO we don't have the input bits until
all 8 bits of the output have been shifted out,
so we have to prime the MISO with bogus values (0xFF).
Signed-off-by: Iris Chen
---
hw/block/m25p80.c| 29 -
hw/ssi/ssi.c | 4
include/hw/ssi/ssi.h | 5 +
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a8d2519141..9b26bb96e5 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1462,7 +1462,7 @@ static int m25p80_cs(SSIPeripheral *ss, bool select)
return 0;
}
-static uint32_t m25p80_transfer8(SSIPeripheral *ss, uint32_t tx)
+static uint32_t m25p80_transfer8_ex(SSIPeripheral *ss, uint32_t tx)
{
Flash *s = M25P80(ss);
uint32_t r = 0;
@@ -1548,6 +1548,33 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss,
uint32_t tx)
return r;
}
+/*
+ * Pre-reading logic for m25p80_transfer8:
+ * The existing SPI model assumes the output byte is fully formed,
+ * can be passed to the SPI device, and the input byte can be returned,
+ * all in one operation. With SPI-GPIO, we don't have the input byte
+ * until all 8 bits of the output have been shifted out, so we have
+ * to prime the MISO with bogus values (0xFF).
+ */
+static uint32_t m25p80_transfer8(SSIPeripheral *ss, uint32_t tx)
+{
+Flash *s = M25P80(ss);
+SSIBus *ssibus = (SSIBus *)qdev_get_parent_bus(DEVICE(s));
+
+uint8_t prev_state = s->state;
+uint32_t r = m25p80_transfer8_ex(ss, tx);
+uint8_t curr_state = s->state;
+
+if (ssibus->preread &&
+ /* cmd state has changed into DATA reading state */
+ (!(prev_state == STATE_READ || prev_state == STATE_READING_DATA) &&
+ (curr_state == STATE_READ || curr_state == STATE_READING_DATA))) {
+r = m25p80_transfer8_ex(ss, 0xFF);
+}
+
+return r;
+}
+
static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int
level)
{
Flash *s = M25P80(opaque);
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index 003931fb50..21570fe25f 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -19,10 +19,6 @@
#include "qapi/error.h"
#include "qom/object.h"
-struct SSIBus {
-BusState parent_obj;
-};
-
#define TYPE_SSI_BUS "SSI"
OBJECT_DECLARE_SIMPLE_TYPE(SSIBus, SSI_BUS)
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index f411858ab0..e54073d822 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -30,6 +30,11 @@ enum SSICSMode {
SSI_CS_HIGH,
};
+struct SSIBus {
+BusState parent_obj;
+bool preread;
+};
+
/* Peripherals. */
struct SSIPeripheralClass {
DeviceClass parent_class;
--
2.30.2
[RFC 0/3] Add Generic SPI GPIO model
Hey everyone, I have been working on a project to add support for SPI-based TPMs in QEMU. Currently, most of our vboot platforms using a SPI-based TPM use the Linux SPI-GPIO driver to "bit-bang" the SPI protocol. This is because the Aspeed SPI controller (modeled in QEMU under hw/ssi/aspeed_smc.c) has an implementation deficiency that prevents bi-directional operations. Thus, in order to connect a TPM to this bus, my patch implements a QEMU SPI-GPIO driver (as the QEMU counterpart of the Linux SPI-GPIO driver). As we use SPI-based TPMs on many of our BMCs for the secure-boot implementation, I have already tested this implementation locally with our Yosemite-v3.5 platform and Facebook-OpenBMC. This model was tested by connecting a generic SPI-NOR (m25p80 for example) to the Yosemite-v3.5 SPI bus containing the TPM. This patch is an RFC because I have several questions about design. Although the model is working, I understand there are many areas where the design decision is not deal (ie. abstracting hard coded GPIO values). Below are some details of the patch and specific areas where I would appreciate feedback on how to make this better: hw/arm/aspeed.c: I am not sure where the best place is to instantiate the spi_gpio besides the aspeed_machine_init. Could we add the ability to instantiate it on the command line? m25p80_transfer8_ex in hw/block/m25p80.c: Existing SPI model assumed that the output byte is fully formed, can be passed to the SPI device, and the input byte can be returned, all in one operation. With SPI-GPIO we don't have the input byte until all 8 bits of the output have been shifted out, so we have to prime the MISO with bogus values (0xFF). MOSI pin in spi_gpio: the mosi pin is not included and we poll the realtime value of the gpio for input bits to prevent bugs with caching the mosi value. It was discovered during testing that when using the mosi pin as the input pin, the mosi value was not being updated due to a kernel and aspeed_gpio model optimization. Thus, here we are reading the value directly from the gpio controller instead of waiting for the push. I realize there are Aspeed specifics in the spi_gpio model. To make this extensible, is it preferred to make this into a base class and have our Aspeed SPI GPIO extend this or we could set up params to pass in the constructor? Thanks for your review and any direction here would be helpful :) Iris Chen (3): hw: m25p80: add prereading ability in transfer8 hw: spi_gpio: add spi gpio model hw: aspeed: hook up the spi gpio model hw/arm/Kconfig| 1 + hw/arm/aspeed.c | 5 ++ hw/block/m25p80.c | 29 ++- hw/ssi/Kconfig| 4 + hw/ssi/meson.build| 1 + hw/ssi/spi_gpio.c | 166 ++ hw/ssi/ssi.c | 4 - include/hw/ssi/spi_gpio.h | 38 + include/hw/ssi/ssi.h | 5 ++ 9 files changed, 248 insertions(+), 5 deletions(-) create mode 100644 hw/ssi/spi_gpio.c create mode 100644 include/hw/ssi/spi_gpio.h -- 2.30.2
[PATCH v3] hw: m25p80: Add Block Protect and Top Bottom bits for write protect
Signed-off-by: Iris Chen
---
Cosmetic suggestions addressed.
hw/block/m25p80.c | 102 --
1 file changed, 90 insertions(+), 12 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 50b523e5b1..f3b401cf90 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -38,21 +38,19 @@
#include "trace.h"
#include "qom/object.h"
-/* Fields for FlashPartInfo->flags */
-
-/* erase capabilities */
-#define ER_4K 1
-#define ER_32K 2
-/* set to allow the page program command to write 0s back to 1. Useful for
- * modelling EEPROM with SPI flash command set
- */
-#define EEPROM 0x100
-
/* 16 MiB max in 3 byte address mode */
#define MAX_3BYTES_SIZE 0x100
-
#define SPI_NOR_MAX_ID_LEN 6
+/* Fields for FlashPartInfo->flags */
+enum spi_flash_option_flags {
+ER_4K = BIT(0),
+ER_32K = BIT(1),
+EEPROM = BIT(2),
+HAS_SR_TB = BIT(3),
+HAS_SR_BP3_BIT6= BIT(4),
+};
+
typedef struct FlashPartInfo {
const char *part_name;
/*
@@ -253,7 +251,8 @@ static const FlashPartInfo known_devices[] = {
{ INFO("n25q512a11", 0x20bb20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512a13", 0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) },
-{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512, ER_4K) },
+{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512,
+ ER_4K | HAS_SR_BP3_BIT6 | HAS_SR_TB) },
{ INFO("n25q512a",0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512ax3", 0x20ba20, 0x1000, 64 << 10, 1024, ER_4K) },
{ INFO("mt25ql512ab", 0x20ba20, 0x1044, 64 << 10, 1024, ER_4K | ER_32K) },
@@ -480,6 +479,11 @@ struct Flash {
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool block_protect0;
+bool block_protect1;
+bool block_protect2;
+bool block_protect3;
+bool top_bottom_bit;
bool status_register_write_disabled;
uint8_t ear;
@@ -625,12 +629,36 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
{
uint32_t page = addr / s->pi->page_size;
uint8_t prev = s->storage[s->cur_addr];
+uint32_t block_protect_value = (s->block_protect3 << 3) |
+ (s->block_protect2 << 2) |
+ (s->block_protect1 << 1) |
+ (s->block_protect0 << 0);
if (!s->write_enable) {
qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
return;
}
+if (block_protect_value > 0) {
+uint32_t num_protected_sectors = 1 << (block_protect_value - 1);
+uint32_t sector = addr / s->pi->sector_size;
+
+/* top_bottom_bit == 0 means TOP */
+if (!s->top_bottom_bit) {
+if (s->pi->n_sectors <= sector + num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+} else {
+if (sector < num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+}
+}
+
if ((prev ^ data) & data) {
trace_m25p80_programming_zero_to_one(s, addr, prev, data);
}
@@ -728,6 +756,15 @@ static void complete_collecting_data(Flash *s)
break;
case WRSR:
s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+s->block_protect0 = extract32(s->data[0], 2, 1);
+s->block_protect1 = extract32(s->data[0], 3, 1);
+s->block_protect2 = extract32(s->data[0], 4, 1);
+if (s->pi->flags & HAS_SR_TB) {
+s->top_bottom_bit = extract32(s->data[0], 5, 1);
+}
+if (s->pi->flags & HAS_SR_BP3_BIT6) {
+s->block_protect3 = extract32(s->data[0], 6, 1);
+}
switch (get_man(s)) {
case MAN_SPANSION:
@@ -1213,6 +1250,15 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
s->data[0] |= (!!s->status_register_write_disabled) << 7;
+s->data[0] |= (!!s->block_protect0) << 2;
+s->data[0] |= (!!s->block_protect1) << 3;
+s->data[0] |= (!!s->block_protect2) << 4;
+if (s->pi->flags & HAS_SR_TB) {
+s->data[0] |= (!!s->top_bottom_bit) << 5;
+}
+
[PATCH v2] hw: m25p80: Add Block Protect and Top Bottom bits for write protect
Signed-off-by: Iris Chen
---
Addressing all comments.
In reponse to this comment:
"Something wrong will occur if all block_protect[0123] are zeroes",
the code actually ignores num_protected_sectors when block_protect_value = 0
which happens when block_protect[0123] are zeroes.
You can refer to the bottom block in v1, where we only look at cases when
block_protect_value > 0 so it is actually handled.
Regardless, since we were setting num_protected_sectors
in either case, I have refactored the code to make it more clear.
hw/block/m25p80.c | 103 --
1 file changed, 91 insertions(+), 12 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 50b523e5b1..bddea9853b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -38,21 +38,19 @@
#include "trace.h"
#include "qom/object.h"
-/* Fields for FlashPartInfo->flags */
-
-/* erase capabilities */
-#define ER_4K 1
-#define ER_32K 2
-/* set to allow the page program command to write 0s back to 1. Useful for
- * modelling EEPROM with SPI flash command set
- */
-#define EEPROM 0x100
-
/* 16 MiB max in 3 byte address mode */
#define MAX_3BYTES_SIZE 0x100
-
#define SPI_NOR_MAX_ID_LEN 6
+/* Fields for FlashPartInfo->flags */
+enum spi_option_flags {
+ER_4K = BIT(0),
+ER_32K = BIT(1),
+EEPROM = BIT(2),
+HAS_SR_TB = BIT(3),
+HAS_SR_BP3_BIT6= BIT(4),
+};
+
typedef struct FlashPartInfo {
const char *part_name;
/*
@@ -253,7 +251,8 @@ static const FlashPartInfo known_devices[] = {
{ INFO("n25q512a11", 0x20bb20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512a13", 0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) },
-{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512, ER_4K) },
+{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512,
+ ER_4K | HAS_SR_BP3_BIT6 | HAS_SR_TB) },
{ INFO("n25q512a",0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512ax3", 0x20ba20, 0x1000, 64 << 10, 1024, ER_4K) },
{ INFO("mt25ql512ab", 0x20ba20, 0x1044, 64 << 10, 1024, ER_4K | ER_32K) },
@@ -480,6 +479,11 @@ struct Flash {
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool block_protect0;
+bool block_protect1;
+bool block_protect2;
+bool block_protect3;
+bool top_bottom_bit;
bool status_register_write_disabled;
uint8_t ear;
@@ -626,11 +630,36 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
uint32_t page = addr / s->pi->page_size;
uint8_t prev = s->storage[s->cur_addr];
+uint32_t block_protect_value = (s->block_protect3 << 3) |
+ (s->block_protect2 << 2) |
+ (s->block_protect1 << 1) |
+ (s->block_protect0 << 0);
+
if (!s->write_enable) {
qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
return;
}
+if (block_protect_value > 0) {
+uint32_t num_protected_sectors = 1 << (block_protect_value - 1);
+uint32_t sector = addr / s->pi->sector_size;
+
+/* top_bottom_bit == 0 means TOP */
+if (!s->top_bottom_bit) {
+if (s->pi->n_sectors <= sector + num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+} else {
+if (sector < num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+}
+}
+
if ((prev ^ data) & data) {
trace_m25p80_programming_zero_to_one(s, addr, prev, data);
}
@@ -728,6 +757,15 @@ static void complete_collecting_data(Flash *s)
break;
case WRSR:
s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+s->block_protect0 = extract32(s->data[0], 2, 1);
+s->block_protect1 = extract32(s->data[0], 3, 1);
+s->block_protect2 = extract32(s->data[0], 4, 1);
+if (s->pi->flags & HAS_SR_TB) {
+s->top_bottom_bit = extract32(s->data[0], 5, 1);
+}
+if (s->pi->flags & HAS_SR_BP3_BIT6) {
+s->block_protect3 = extract32(s->data[0], 6, 1);
+}
switch (get_man(s)) {
case MAN_SPANSION:
@@ -1213,6 +1251,15 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case
[PATCH 1/2] hw: m25p80: Add Block Protect and Top Bottom bits for write protect
Signed-off-by: Iris Chen
---
hw/block/m25p80.c | 74 +++
1 file changed, 62 insertions(+), 12 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 50b523e5b1..0156a70f5e 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -38,21 +38,19 @@
#include "trace.h"
#include "qom/object.h"
-/* Fields for FlashPartInfo->flags */
-
-/* erase capabilities */
-#define ER_4K 1
-#define ER_32K 2
-/* set to allow the page program command to write 0s back to 1. Useful for
- * modelling EEPROM with SPI flash command set
- */
-#define EEPROM 0x100
-
/* 16 MiB max in 3 byte address mode */
#define MAX_3BYTES_SIZE 0x100
-
#define SPI_NOR_MAX_ID_LEN 6
+/* Fields for FlashPartInfo->flags */
+enum spi_nor_option_flags {
+ER_4K = BIT(0),
+ER_32K = BIT(1),
+EEPROM = BIT(2),
+SNOR_F_HAS_SR_TB = BIT(3),
+SNOR_F_HAS_SR_BP3_BIT6 = BIT(4),
+};
+
typedef struct FlashPartInfo {
const char *part_name;
/*
@@ -253,7 +251,8 @@ static const FlashPartInfo known_devices[] = {
{ INFO("n25q512a11", 0x20bb20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512a13", 0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) },
-{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512, ER_4K) },
+{ INFO("n25q256a",0x20ba19, 0, 64 << 10, 512,
+ ER_4K | SNOR_F_HAS_SR_BP3_BIT6 | SNOR_F_HAS_SR_TB) },
{ INFO("n25q512a",0x20ba20, 0, 64 << 10, 1024, ER_4K) },
{ INFO("n25q512ax3", 0x20ba20, 0x1000, 64 << 10, 1024, ER_4K) },
{ INFO("mt25ql512ab", 0x20ba20, 0x1044, 64 << 10, 1024, ER_4K | ER_32K) },
@@ -480,6 +479,11 @@ struct Flash {
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool block_protect0;
+bool block_protect1;
+bool block_protect2;
+bool block_protect3;
+bool top_bottom_bit;
bool status_register_write_disabled;
uint8_t ear;
@@ -630,6 +634,29 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
return;
}
+uint32_t block_protect_value = (s->block_protect3 << 3) |
+ (s->block_protect2 << 2) |
+ (s->block_protect1 << 1) |
+ (s->block_protect0 << 0);
+
+ uint32_t num_protected_sectors = 1 << (block_protect_value - 1);
+ uint32_t sector = addr / s->pi->sector_size;
+
+ /* top_bottom_bit == 0 means TOP */
+if (!s->top_bottom_bit) {
+if (block_protect_value > 0 &&
+s->pi->n_sectors <= sector + num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+} else {
+if (block_protect_value > 0 && sector < num_protected_sectors) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: write with write protect!\n");
+return;
+}
+}
if ((prev ^ data) & data) {
trace_m25p80_programming_zero_to_one(s, addr, prev, data);
@@ -728,6 +755,15 @@ static void complete_collecting_data(Flash *s)
break;
case WRSR:
s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+s->block_protect0 = extract32(s->data[0], 2, 1);
+s->block_protect1 = extract32(s->data[0], 3, 1);
+s->block_protect2 = extract32(s->data[0], 4, 1);
+if (s->pi->flags & SNOR_F_HAS_SR_TB) {
+s->top_bottom_bit = extract32(s->data[0], 5, 1);
+}
+if (s->pi->flags & SNOR_F_HAS_SR_BP3_BIT6) {
+s->block_protect3 = extract32(s->data[0], 6, 1);
+}
switch (get_man(s)) {
case MAN_SPANSION:
@@ -1213,6 +1249,15 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
s->data[0] |= (!!s->status_register_write_disabled) << 7;
+s->data[0] |= (!!s->block_protect0) << 2;
+s->data[0] |= (!!s->block_protect1) << 3;
+s->data[0] |= (!!s->block_protect2) << 4;
+if (s->pi->flags & SNOR_F_HAS_SR_TB) {
+s->data[0] |= (!!s->top_bottom_bit) << 5;
+}
+if (s->pi->flags & SNOR_F_HAS_SR_BP3_BIT6) {
+s->data[0] |= (!!s->block_protect3) << 6;
+}
if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
s->data[0] |= (!!s->quad_enable) << 6;
@@ -1553,6 +1598,11 @@ static void m25p80_reset(DeviceState *d)
s->wp_level = true;
s->status_register_write_disabled = false;
+s->block_protect0 = false;
+s->block_protect1 = false;
+s->block_protect2 = false;
+s->block_protect3 = false;
+s->top_bottom_bit = false;
reset_memory(s);
}
--
2.30.2
[PATCH 2/2] hw: m25p80: add tests for BP and TB bit write protect
Signed-off-by: Iris Chen
---
tests/qtest/aspeed_smc-test.c | 111 ++
1 file changed, 111 insertions(+)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 1258687eac..05ce941566 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -192,6 +192,24 @@ static void read_page_mem(uint32_t addr, uint32_t *page)
}
}
+static void write_page_mem(uint32_t addr, uint32_t write_value)
+{
+spi_ctrl_setmode(CTRL_WRITEMODE, PP);
+
+for (int i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+writel(ASPEED_FLASH_BASE + addr + i * 4, write_value);
+}
+}
+
+static void assert_page_mem(uint32_t addr, uint32_t expected_value)
+{
+uint32_t page[FLASH_PAGE_SIZE / 4];
+read_page_mem(addr, page);
+for (int i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+g_assert_cmphex(page[i], ==, expected_value);
+}
+}
+
static void test_erase_sector(void)
{
uint32_t some_page_addr = 0x600 * FLASH_PAGE_SIZE;
@@ -501,6 +519,95 @@ static void test_status_reg_write_protection(void)
flash_reset();
}
+static void test_write_block_protect(void)
+{
+uint32_t sector_size = 65536;
+uint32_t n_sectors = 512;
+
+spi_ce_ctrl(1 << CRTL_EXTENDED0);
+spi_conf(CONF_ENABLE_W0);
+
+uint32_t bp_bits = 0b0;
+
+for (int i = 0; i < 16; i++) {
+bp_bits = ((i & 0b1000) << 3) | ((i & 0b0111) << 2);
+
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, BULK_ERASE);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, bp_bits);
+writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
+writeb(ASPEED_FLASH_BASE, WREN);
+spi_ctrl_stop_user();
+
+uint32_t num_protected_sectors = i ? MIN(1 << (i - 1), n_sectors) : 0;
+uint32_t protection_start = n_sectors - num_protected_sectors;
+uint32_t protection_end = n_sectors;
+
+for (int sector = 0; sector < n_sectors; sector++) {
+uint32_t addr = sector * sector_size;
+
+assert_page_mem(addr, 0x);
+write_page_mem(addr, make_be32(0xabcdef12));
+
+uint32_t expected_value = protection_start <= sector
+ && sector < protection_end
+ ? 0x : 0xabcdef12;
+
+assert_page_mem(addr, expected_value);
+}
+}
+
+flash_reset();
+}
+
+static void test_write_block_protect_bottom_bit(void)
+{
+uint32_t sector_size = 65536;
+uint32_t n_sectors = 512;
+
+spi_ce_ctrl(1 << CRTL_EXTENDED0);
+spi_conf(CONF_ENABLE_W0);
+
+/* top bottom bit is enabled */
+uint32_t bp_bits = 0b00100 << 3;
+
+for (int i = 0; i < 16; i++) {
+bp_bits = (((i & 0b1000) | 0b0100) << 3) | ((i & 0b0111) << 2);
+
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, BULK_ERASE);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, bp_bits);
+writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
+writeb(ASPEED_FLASH_BASE, WREN);
+spi_ctrl_stop_user();
+
+uint32_t num_protected_sectors = i ? MIN(1 << (i - 1), n_sectors) : 0;
+uint32_t protection_start = 0;
+uint32_t protection_end = num_protected_sectors;
+
+for (int sector = 0; sector < n_sectors; sector++) {
+uint32_t addr = sector * sector_size;
+
+assert_page_mem(addr, 0x);
+write_page_mem(addr, make_be32(0xabcdef12));
+
+uint32_t expected_value = protection_start <= sector
+ && sector < protection_end
+ ? 0x : 0xabcdef12;
+
+assert_page_mem(addr, expected_value);
+}
+}
+
+flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -529,6 +636,10 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
qtest_add_func("/ast2400/smc/status_reg_write_protection",
test_status_reg_write_protection);
+qtest_add_func("/ast2400/smc/write_block_protect",
+ test_write_block_protect);
+qtest_add_func("/ast2400/smc/write_block_protect_bottom_bit",
+ test_write_block_protect_bottom_bit);
flash_reset();
ret = g_test_run();
--
2.30.2
[PATCH 0/2] Add Block Protect (BP) and Top Bottom (TB) bits for write protect
Hey everyone, Adding the 3 block protection bits and top bottom bits which configure which parts of the flash are writable and read-only. These patches are based on previous patches I just submitted: hw: m25p80: fixing individual test failure when tests are running in isolation hw: m25p80: add WP# pin and SRWD bit for write protection hw: m25p80: add tests for write protect (WP# and SRWD bit) The tests that were added iterates through every different memory protection case so it takes a little longer to run. Let me know what you think about that and if that's okay or not. Thanks, Iris Iris Chen (2): hw: m25p80: Add Block Protect and Top Bottom bits for write protect hw: m25p80: add tests for BP and TB bit write protect hw/block/m25p80.c | 74 +++ tests/qtest/aspeed_smc-test.c | 111 ++ 2 files changed, 173 insertions(+), 12 deletions(-) -- 2.30.2
[PATCH v4] hw: m25p80: add tests for write protect (WP# and SRWD bit)
Signed-off-by: Iris Chen
---
Adding Signed Off By tag -- sorry I missed that !
tests/qtest/aspeed_smc-test.c | 62 +++
1 file changed, 62 insertions(+)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index ec233315e6..7786addfb8 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -56,7 +56,9 @@ enum {
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
+WRSR = 0x1,
WREN = 0x6,
+SRWD = 0x80,
RESET_ENABLE = 0x66,
RESET_MEMORY = 0x99,
EN_4BYTE_ADDR = 0xB7,
@@ -390,6 +392,64 @@ static void test_read_status_reg(void)
flash_reset();
}
+static void test_status_reg_write_protection(void)
+{
+uint8_t r;
+
+spi_conf(CONF_ENABLE_W0);
+
+/* default case: WP# is high and SRWD is low -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# high and SRWD high -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, 0);
+
+/* WP# low and SRWD low -> status register writable */
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 0);
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# low and SRWD high -> status register NOT writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+/* write is not successful */
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 1);
+flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -416,6 +476,8 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
+qtest_add_func("/ast2400/smc/status_reg_write_protection",
+ test_status_reg_write_protection);
ret = g_test_run();
--
2.30.2
[PATCH v4] hw: m25p80: add WP# pin and SRWD bit for write protection
From: Iris Chen
Signed-off-by: Iris Chen
---
Fixed .needed for subsection and suggestions from Francisco
hw/block/m25p80.c | 82 ++-
1 file changed, 67 insertions(+), 15 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 81ba3da4df..3045dda53b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -472,11 +472,13 @@ struct Flash {
uint8_t spansion_cr2v;
uint8_t spansion_cr3v;
uint8_t spansion_cr4v;
+bool wp_level;
bool write_enable;
bool four_bytes_address_mode;
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool status_register_write_disabled;
uint8_t ear;
int64_t dirty_page;
@@ -723,6 +725,8 @@ static void complete_collecting_data(Flash *s)
flash_erase(s, s->cur_addr, s->cmd_in_progress);
break;
case WRSR:
+s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+
switch (get_man(s)) {
case MAN_SPANSION:
s->quad_enable = !!(s->data[1] & 0x02);
@@ -1165,22 +1169,34 @@ static void decode_new_cmd(Flash *s, uint32_t value)
break;
case WRSR:
-if (s->write_enable) {
-switch (get_man(s)) {
-case MAN_SPANSION:
-s->needed_bytes = 2;
-s->state = STATE_COLLECTING_DATA;
-break;
-case MAN_MACRONIX:
-s->needed_bytes = 2;
-s->state = STATE_COLLECTING_VAR_LEN_DATA;
-break;
-default:
-s->needed_bytes = 1;
-s->state = STATE_COLLECTING_DATA;
-}
-s->pos = 0;
+/*
+ * If WP# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+if ((s->wp_level == 0 && s->status_register_write_disabled)
+|| !s->write_enable) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Status register write is disabled!\n");
+break;
+}
+
+switch (get_man(s)) {
+case MAN_SPANSION:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_DATA;
+break;
+case MAN_MACRONIX:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
+default:
+s->needed_bytes = 1;
+s->state = STATE_COLLECTING_DATA;
}
+s->pos = 0;
break;
case WRDI:
@@ -1195,6 +1211,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
+s->data[0] |= (!!s->status_register_write_disabled) << 7;
+
if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
s->data[0] |= (!!s->quad_enable) << 6;
}
@@ -1484,6 +1502,14 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss,
uint32_t tx)
return r;
}
+static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int
level)
+{
+Flash *s = M25P80(opaque);
+/* WP# is just a single pin. */
+assert(n == 0);
+s->wp_level = !!level;
+}
+
static void m25p80_realize(SSIPeripheral *ss, Error **errp)
{
Flash *s = M25P80(ss);
@@ -1515,12 +1541,18 @@ static void m25p80_realize(SSIPeripheral *ss, Error
**errp)
s->storage = blk_blockalign(NULL, s->size);
memset(s->storage, 0xFF, s->size);
}
+
+qdev_init_gpio_in_named(DEVICE(s),
+m25p80_write_protect_pin_irq_handler, "WP#", 1);
}
static void m25p80_reset(DeviceState *d)
{
Flash *s = M25P80(d);
+s->wp_level = true;
+s->status_register_write_disabled = false;
+
reset_memory(s);
}
@@ -1587,6 +1619,25 @@ static const VMStateDescription
vmstate_m25p80_aai_enable = {
}
};
+static bool m25p80_wp_level_srwd_needed(void *opaque)
+{
+Flash *s = (Flash *)opaque;
+
+return !s->wp_level || s->status_register_write_disabled;
+}
+
+static const VMStateDescription vmstate_m25p80_write_protect = {
+.name = "m25p80/write_protect",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = m25p80_wp_level_srwd_needed,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(wp_level, Flash),
+VMSTATE_BOOL(status_register_write_disabled, Flash),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_m25p80 = {
.name = "m25p80",
.version_id = 0,
@@ -1618,6 +1669,7 @@ stati
[PATCH 1/1] hw: m25p80: fixing individual test failure when tests are running in isolation
Signed-off-by: Iris Chen
---
tests/qtest/aspeed_smc-test.c | 74 +--
1 file changed, 63 insertions(+), 11 deletions(-)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index ec233315e6..b1e682db65 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -135,6 +135,9 @@ static void flash_reset(void)
spi_ctrl_start_user();
writeb(ASPEED_FLASH_BASE, RESET_ENABLE);
writeb(ASPEED_FLASH_BASE, RESET_MEMORY);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, BULK_ERASE);
+writeb(ASPEED_FLASH_BASE, WRDI);
spi_ctrl_stop_user();
spi_conf_remove(CONF_ENABLE_W0);
@@ -195,21 +198,41 @@ static void test_erase_sector(void)
spi_conf(CONF_ENABLE_W0);
+/*
+ * Previous page should be full of 0xffs after backend is
+ * initialized
+ */
+read_page(some_page_addr - FLASH_PAGE_SIZE, page);
+for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+g_assert_cmphex(page[i], ==, 0x);
+}
+
spi_ctrl_start_user();
-writeb(ASPEED_FLASH_BASE, WREN);
writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
-writeb(ASPEED_FLASH_BASE, ERASE_SECTOR);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, PP);
writel(ASPEED_FLASH_BASE, make_be32(some_page_addr));
+
+/* Fill the page with its own addresses */
+for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+writel(ASPEED_FLASH_BASE, make_be32(some_page_addr + i * 4));
+}
spi_ctrl_stop_user();
-/* Previous page should be full of zeroes as backend is not
- * initialized */
-read_page(some_page_addr - FLASH_PAGE_SIZE, page);
+/* Check the page is correctly written */
+read_page(some_page_addr, page);
for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
-g_assert_cmphex(page[i], ==, 0x0);
+g_assert_cmphex(page[i], ==, some_page_addr + i * 4);
}
-/* But this one was erased */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
+writeb(ASPEED_FLASH_BASE, ERASE_SECTOR);
+writel(ASPEED_FLASH_BASE, make_be32(some_page_addr));
+spi_ctrl_stop_user();
+
+/* Check the page is erased */
read_page(some_page_addr, page);
for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
g_assert_cmphex(page[i], ==, 0x);
@@ -226,11 +249,31 @@ static void test_erase_all(void)
spi_conf(CONF_ENABLE_W0);
-/* Check some random page. Should be full of zeroes as backend is
- * not initialized */
+/*
+ * Previous page should be full of 0xffs after backend is
+ * initialized
+ */
+read_page(some_page_addr - FLASH_PAGE_SIZE, page);
+for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+g_assert_cmphex(page[i], ==, 0x);
+}
+
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, PP);
+writel(ASPEED_FLASH_BASE, make_be32(some_page_addr));
+
+/* Fill the page with its own addresses */
+for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+writel(ASPEED_FLASH_BASE, make_be32(some_page_addr + i * 4));
+}
+spi_ctrl_stop_user();
+
+/* Check the page is correctly written */
read_page(some_page_addr, page);
for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
-g_assert_cmphex(page[i], ==, 0x0);
+g_assert_cmphex(page[i], ==, some_page_addr + i * 4);
}
spi_ctrl_start_user();
@@ -238,7 +281,7 @@ static void test_erase_all(void)
writeb(ASPEED_FLASH_BASE, BULK_ERASE);
spi_ctrl_stop_user();
-/* Recheck that some random page */
+/* Check the page is erased */
read_page(some_page_addr, page);
for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
g_assert_cmphex(page[i], ==, 0x);
@@ -299,6 +342,14 @@ static void test_read_page_mem(void)
spi_conf(CONF_ENABLE_W0);
spi_ctrl_start_user();
writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR);
+writeb(ASPEED_FLASH_BASE, WREN);
+writeb(ASPEED_FLASH_BASE, PP);
+writel(ASPEED_FLASH_BASE, make_be32(my_page_addr));
+
+/* Fill the page with its own addresses */
+for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+writel(ASPEED_FLASH_BASE, make_be32(my_page_addr + i * 4));
+}
spi_ctrl_stop_user();
spi_conf_remove(CONF_ENABLE_W0);
@@ -417,6 +468,7 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
+flash_reset();
ret = g_test_run();
qtest_quit(global_qtest);
--
2.30.2
[PATCH 0/1] hw: m25p80: fix aspeed_smc tests failure when run in isolation
Hey everyone, I discovered that some of the tests in tests/qtest/aspeed_smc-test.c were failing when run in isolation due to dependencies between the tests. For example, one test would test the reading of a block of memory written in the test before it. I think it would make sense to add flash_reset() between running the tests and make sure the tests do not rely on each other. Thus, I have made changes to the tests so that they now pass individually with no dependencies on each other. Thanks, Iris Iris Chen (1): hw: m25p80: fixing individual test failure when tests are running in isolation tests/qtest/aspeed_smc-test.c | 74 +-- 1 file changed, 63 insertions(+), 11 deletions(-) -- 2.30.2
[PATCH v3 2/2] hw: m25p80: add tests for write protect (WP# and SRWD bit)
---
Fixing suggestions to move testing related code to a different commit.
tests/qtest/aspeed_smc-test.c | 62 +++
1 file changed, 62 insertions(+)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index ec233315e6..7786addfb8 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -56,7 +56,9 @@ enum {
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
+WRSR = 0x1,
WREN = 0x6,
+SRWD = 0x80,
RESET_ENABLE = 0x66,
RESET_MEMORY = 0x99,
EN_4BYTE_ADDR = 0xB7,
@@ -390,6 +392,64 @@ static void test_read_status_reg(void)
flash_reset();
}
+static void test_status_reg_write_protection(void)
+{
+uint8_t r;
+
+spi_conf(CONF_ENABLE_W0);
+
+/* default case: WP# is high and SRWD is low -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# high and SRWD high -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, 0);
+
+/* WP# low and SRWD low -> status register writable */
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 0);
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# low and SRWD high -> status register NOT writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+/* write is not successful */
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 1);
+flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -416,6 +476,8 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
+qtest_add_func("/ast2400/smc/status_reg_write_protection",
+ test_status_reg_write_protection);
ret = g_test_run();
--
2.30.2
[PATCH v3 1/2] hw: m25p80: add WP# pin and SRWD bit for write protection
Signed-off-by: Iris Chen
---
Thanks everyone for your comments. This is a v3 patch that addresses all
suggestions (moving write_enable to decode_new_cmd).
I am waiting on some feedback from Dan's (dz4l...@gmail.com) patch
regarding adding a STATE_STANDBY state.
Currently, all tests are passing.
hw/block/m25p80.c | 77 ++-
1 file changed, 62 insertions(+), 15 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 81ba3da4df..12a59ca57c 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -27,12 +27,14 @@
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
#include "hw/ssi/ssi.h"
+#include "hw/irq.h"
#include "migration/vmstate.h"
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "trace.h"
#include "qom/object.h"
@@ -472,11 +474,13 @@ struct Flash {
uint8_t spansion_cr2v;
uint8_t spansion_cr3v;
uint8_t spansion_cr4v;
+bool wp_level;
bool write_enable;
bool four_bytes_address_mode;
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool status_register_write_disabled;
uint8_t ear;
int64_t dirty_page;
@@ -723,6 +727,8 @@ static void complete_collecting_data(Flash *s)
flash_erase(s, s->cur_addr, s->cmd_in_progress);
break;
case WRSR:
+s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+
switch (get_man(s)) {
case MAN_SPANSION:
s->quad_enable = !!(s->data[1] & 0x02);
@@ -1165,22 +1171,34 @@ static void decode_new_cmd(Flash *s, uint32_t value)
break;
case WRSR:
-if (s->write_enable) {
-switch (get_man(s)) {
-case MAN_SPANSION:
-s->needed_bytes = 2;
-s->state = STATE_COLLECTING_DATA;
-break;
-case MAN_MACRONIX:
-s->needed_bytes = 2;
-s->state = STATE_COLLECTING_VAR_LEN_DATA;
-break;
-default:
-s->needed_bytes = 1;
-s->state = STATE_COLLECTING_DATA;
-}
-s->pos = 0;
+/*
+ * If WP# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+if ((s->wp_level == 0 && s->status_register_write_disabled)
+|| !s->write_enable) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Status register write is disabled!\n");
+break;
}
+
+switch (get_man(s)) {
+case MAN_SPANSION:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_DATA;
+break;
+case MAN_MACRONIX:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
+default:
+s->needed_bytes = 1;
+s->state = STATE_COLLECTING_DATA;
+}
+s->pos = 0;
break;
case WRDI:
@@ -1195,6 +1213,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
+s->data[0] |= (!!s->status_register_write_disabled) << 7;
+
if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
s->data[0] |= (!!s->quad_enable) << 6;
}
@@ -1484,6 +1504,14 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss,
uint32_t tx)
return r;
}
+static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int
level)
+{
+Flash *s = M25P80(opaque);
+/* WP# is just a single pin. */
+assert(n == 0);
+s->wp_level = !!level;
+}
+
static void m25p80_realize(SSIPeripheral *ss, Error **errp)
{
Flash *s = M25P80(ss);
@@ -1515,12 +1543,18 @@ static void m25p80_realize(SSIPeripheral *ss, Error
**errp)
s->storage = blk_blockalign(NULL, s->size);
memset(s->storage, 0xFF, s->size);
}
+
+qdev_init_gpio_in_named(DEVICE(s),
+m25p80_write_protect_pin_irq_handler, "WP#", 1);
}
static void m25p80_reset(DeviceState *d)
{
Flash *s = M25P80(d);
+s->wp_level = true;
+s->status_register_write_disabled = false;
+
reset_memory(s);
}
@@ -1587,6 +1621,18 @@ static const VMStateDescription
vmstate_m25p80_aai_enable = {
[PATCH v2 2/2] hw: m25p80: add tests for write protect
Signed-off-by: Iris Chen
---
Include the tests in a separate patch. Using qtest_set_irq_in() as per review.
tests/qtest/aspeed_smc-test.c | 60 +++
1 file changed, 60 insertions(+)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index c5d97d4410..7786addfb8 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -392,6 +392,64 @@ static void test_read_status_reg(void)
flash_reset();
}
+static void test_status_reg_write_protection(void)
+{
+uint8_t r;
+
+spi_conf(CONF_ENABLE_W0);
+
+/* default case: WP# is high and SRWD is low -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# high and SRWD high -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, 0);
+
+/* WP# low and SRWD low -> status register writable */
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 0);
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# low and SRWD high -> status register NOT writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+/* write is not successful */
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 1);
+flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -418,6 +476,8 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
+qtest_add_func("/ast2400/smc/status_reg_write_protection",
+ test_status_reg_write_protection);
ret = g_test_run();
--
2.30.2
[PATCH v2 1/2] hw: m25p80: add WP# pin and SRWD bit for write protection
From: Iris Chen
Signed-off-by: Iris Chen
---
Addressed all comments from V1. The biggest change: removed
object_class_property_add.
hw/block/m25p80.c | 37 +++
tests/qtest/aspeed_smc-test.c | 2 ++
2 files changed, 39 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 81ba3da4df..1a20bd55d4 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -27,12 +27,14 @@
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
#include "hw/ssi/ssi.h"
+#include "hw/irq.h"
#include "migration/vmstate.h"
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "trace.h"
#include "qom/object.h"
@@ -472,11 +474,13 @@ struct Flash {
uint8_t spansion_cr2v;
uint8_t spansion_cr3v;
uint8_t spansion_cr4v;
+bool wp_level;
bool write_enable;
bool four_bytes_address_mode;
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool status_register_write_disabled;
uint8_t ear;
int64_t dirty_page;
@@ -723,6 +727,21 @@ static void complete_collecting_data(Flash *s)
flash_erase(s, s->cur_addr, s->cmd_in_progress);
break;
case WRSR:
+/*
+ * If WP# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+if ((s->wp_level == 0 && s->status_register_write_disabled)
+|| !s->write_enable) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Status register write is disabled!\n");
+break;
+}
+s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+
switch (get_man(s)) {
case MAN_SPANSION:
s->quad_enable = !!(s->data[1] & 0x02);
@@ -1195,6 +1214,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
+s->data[0] |= (!!s->status_register_write_disabled) << 7;
+
if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
s->data[0] |= (!!s->quad_enable) << 6;
}
@@ -1484,6 +1505,14 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss,
uint32_t tx)
return r;
}
+static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int
level)
+{
+Flash *s = M25P80(opaque);
+/* WP# is just a single pin. */
+assert(n == 0);
+s->wp_level = !!level;
+}
+
static void m25p80_realize(SSIPeripheral *ss, Error **errp)
{
Flash *s = M25P80(ss);
@@ -1515,12 +1544,18 @@ static void m25p80_realize(SSIPeripheral *ss, Error
**errp)
s->storage = blk_blockalign(NULL, s->size);
memset(s->storage, 0xFF, s->size);
}
+
+qdev_init_gpio_in_named(DEVICE(s),
+m25p80_write_protect_pin_irq_handler, "WP#", 1);
}
static void m25p80_reset(DeviceState *d)
{
Flash *s = M25P80(d);
+s->wp_level = true;
+s->status_register_write_disabled = false;
+
reset_memory(s);
}
@@ -1601,6 +1636,8 @@ static const VMStateDescription vmstate_m25p80 = {
VMSTATE_UINT8(needed_bytes, Flash),
VMSTATE_UINT8(cmd_in_progress, Flash),
VMSTATE_UINT32(cur_addr, Flash),
+VMSTATE_BOOL(wp_level, Flash),
+VMSTATE_BOOL(status_register_write_disabled, Flash),
VMSTATE_BOOL(write_enable, Flash),
VMSTATE_BOOL(reset_enable, Flash),
VMSTATE_UINT8(ear, Flash),
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index ec233315e6..c5d97d4410 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -56,7 +56,9 @@ enum {
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
+WRSR = 0x1,
WREN = 0x6,
+SRWD = 0x80,
RESET_ENABLE = 0x66,
RESET_MEMORY = 0x99,
EN_4BYTE_ADDR = 0xB7,
--
2.30.2
[PATCH 1/1] hw: m25p80: add W# pin and SRWD bit for write protection
From: Iris Chen
Add the W# pin and SRWD bit which control the status register write
ability.
Signed-off-by: Iris Chen
---
hw/block/m25p80.c | 72 +++
tests/qtest/aspeed_smc-test.c | 62 ++
2 files changed, 134 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 81ba3da4df..c845fa08d4 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -27,12 +27,14 @@
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
#include "hw/ssi/ssi.h"
+#include "hw/irq.h"
#include "migration/vmstate.h"
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "trace.h"
#include "qom/object.h"
@@ -472,11 +474,13 @@ struct Flash {
uint8_t spansion_cr2v;
uint8_t spansion_cr3v;
uint8_t spansion_cr4v;
+bool write_protect_pin;
bool write_enable;
bool four_bytes_address_mode;
bool reset_enable;
bool quad_enable;
bool aai_enable;
+bool status_register_write_disabled;
uint8_t ear;
int64_t dirty_page;
@@ -723,6 +727,21 @@ static void complete_collecting_data(Flash *s)
flash_erase(s, s->cur_addr, s->cmd_in_progress);
break;
case WRSR:
+/*
+ * If W# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+if ((s->write_protect_pin == 0 && s->status_register_write_disabled)
+|| !s->write_enable) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Status register write is disabled!\n");
+break;
+}
+s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+
switch (get_man(s)) {
case MAN_SPANSION:
s->quad_enable = !!(s->data[1] & 0x02);
@@ -1195,6 +1214,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RDSR:
s->data[0] = (!!s->write_enable) << 1;
+s->data[0] |= (!!s->status_register_write_disabled) << 7;
+
if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
s->data[0] |= (!!s->quad_enable) << 6;
}
@@ -1484,6 +1505,15 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss,
uint32_t tx)
return r;
}
+static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int
level)
+{
+Flash *s = M25P80(opaque);
+bool wp = !!level;
+/* W# is just a single pin. */
+assert(n == 0);
+s->write_protect_pin = wp;
+}
+
static void m25p80_realize(SSIPeripheral *ss, Error **errp)
{
Flash *s = M25P80(ss);
@@ -1515,12 +1545,18 @@ static void m25p80_realize(SSIPeripheral *ss, Error
**errp)
s->storage = blk_blockalign(NULL, s->size);
memset(s->storage, 0xFF, s->size);
}
+
+qdev_init_gpio_in_named(DEVICE(s),
+m25p80_write_protect_pin_irq_handler, "W#", 1);
}
static void m25p80_reset(DeviceState *d)
{
Flash *s = M25P80(d);
+s->write_protect_pin = true;
+s->status_register_write_disabled = false;
+
reset_memory(s);
}
@@ -1601,6 +1637,7 @@ static const VMStateDescription vmstate_m25p80 = {
VMSTATE_UINT8(needed_bytes, Flash),
VMSTATE_UINT8(cmd_in_progress, Flash),
VMSTATE_UINT32(cur_addr, Flash),
+VMSTATE_BOOL(write_protect_pin, Flash),
VMSTATE_BOOL(write_enable, Flash),
VMSTATE_BOOL(reset_enable, Flash),
VMSTATE_UINT8(ear, Flash),
@@ -1622,6 +1659,38 @@ static const VMStateDescription vmstate_m25p80 = {
}
};
+static void m25p80_get_write_protect_pin(Object *obj,
+ Visitor *v,
+ const char *name,
+ void *opaque,
+ Error **errp)
+{
+Flash *s = M25P80(obj);
+bool value;
+
+value = s->write_protect_pin;
+
+visit_type_bool(v, name, , errp);
+}
+
+static void m25p80_set_write_protect_pin(Object *obj,
+ Visitor *v,
+ const char *name,
+ void *opaque,
+ Error **errp)
+{
+Flash *s = M25P80(obj);
+bool value;
+qemu_irq w;
+
+if (!visit_type_bool(v, name, , errp)) {
+return;
+}
+
+w = qd
[PATCH 0/1] hw: m25p80: add W# pin and SRWD bit for write protection
From: Iris Chen Hey everyone, My patch adds the W# pin and SRWD bit which work together to control the status register write ability. Accordingly, when W# is low and SRWD bit is high, hardware protection mode (HPM) is initiated. All other cases result in software protection. Acceptance tests have been added to verify all four scenarios: it tests the ability to write to SRWD depending on whether write protection is set. Thanks, Iris Iris Chen (1): hw: m25p80: add W# pin and SRWD bit for write protection hw/block/m25p80.c | 72 +++ tests/qtest/aspeed_smc-test.c | 62 ++ 2 files changed, 134 insertions(+) -- 2.30.2
[PATCH v3] hw: m25p80: allow write_enable latch get/set
The write_enable latch property is not currently exposed.
This commit makes it a modifiable property.
Signed-off-by: Iris Chen
---
v3: Addressed comments by Peter and Cedric.
v2: Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed
comments regarding DEFINE_PROP_BOOL.
hw/block/m25p80.c | 1 +
tests/qtest/aspeed_gpio-test.c | 40 +++
tests/qtest/aspeed_smc-test.c | 43 ++
tests/qtest/libqtest.c | 24 +++
tests/qtest/libqtest.h | 22 +
5 files changed, 98 insertions(+), 32 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 430d1298a8..4283b990af 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1558,6 +1558,7 @@ static int m25p80_pre_save(void *opaque)
static Property m25p80_properties[] = {
/* This is default value for Micron flash */
+DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false),
DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index c1003f2d1b..bac63e8742 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -28,30 +28,6 @@
#include "qapi/qmp/qdict.h"
#include "libqtest-single.h"
-static bool qom_get_bool(QTestState *s, const char *path, const char *property)
-{
-QDict *r;
-bool b;
-
-r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
- "{ 'path': %s, 'property': %s } }", path, property);
-b = qdict_get_bool(r, "return");
-qobject_unref(r);
-
-return b;
-}
-
-static void qom_set_bool(QTestState *s, const char *path, const char *property,
- bool value)
-{
-QDict *r;
-
-r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
- "{ 'path': %s, 'property': %s, 'value': %i } }",
- path, property, value);
-qobject_unref(r);
-}
-
static void test_set_colocated_pins(const void *data)
{
QTestState *s = (QTestState *)data;
@@ -60,14 +36,14 @@ static void test_set_colocated_pins(const void *data)
* gpioV4-7 occupy bits within a single 32-bit value, so we want to make
* sure that modifying one doesn't affect the other.
*/
-qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
-qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
-qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
-qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
-g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
-g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
-g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
-g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
+qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
+qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
+qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
+qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
+g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
+g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
+g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
+g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
}
int main(int argc, char **argv)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 87b40a0ef1..ec233315e6 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "qemu/bswap.h"
#include "libqtest-single.h"
+#include "qemu/bitops.h"
/*
* ASPEED SPI Controller registers
@@ -40,6 +41,7 @@
#define CTRL_FREADMODE 0x1
#define CTRL_WRITEMODE 0x2
#define CTRL_USERMODE0x3
+#define SR_WEL BIT(1)
#define ASPEED_FMC_BASE0x1E62
#define ASPEED_FLASH_BASE 0x2000
@@ -49,6 +51,8 @@
*/
enum {
JEDEC_READ = 0x9f,
+RDSR = 0x5,
+WRDI = 0x4,
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
@@ -348,6 +352,44 @@ static void test_write_page_mem(void)
flash_reset();
}
+static void test_read_status_reg(void)
+{
+uint8_t r;
+
+spi_conf(CONF_ENABLE_W0);
+
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, RDSR)
[PATCH v2] hw: m25p80: allow write_enable latch get/set
The write_enable latch property is not currently exposed.
This commit makes it a modifiable property using get/set methods.
Signed-off-by: Iris Chen
---
Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed
comments regarding DEFINE_PROP_BOOL.
hw/block/m25p80.c | 2 ++
tests/qtest/aspeed_smc-test.c | 23 +++
2 files changed, 25 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 430d1298a8..019beb5b78 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -35,6 +35,7 @@
#include "qapi/error.h"
#include "trace.h"
#include "qom/object.h"
+#include "qapi/visitor.h"
/* Fields for FlashPartInfo->flags */
@@ -1558,6 +1559,7 @@ static int m25p80_pre_save(void *opaque)
static Property m25p80_properties[] = {
/* This is default value for Micron flash */
+DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false),
DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 87b40a0ef1..fcc156bc00 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "qemu/bswap.h"
#include "libqtest-single.h"
+#include "qemu/bitops.h"
/*
* ASPEED SPI Controller registers
@@ -40,6 +41,7 @@
#define CTRL_FREADMODE 0x1
#define CTRL_WRITEMODE 0x2
#define CTRL_USERMODE0x3
+#define SR_WEL BIT(1)
#define ASPEED_FMC_BASE0x1E62
#define ASPEED_FLASH_BASE 0x2000
@@ -49,6 +51,7 @@
*/
enum {
JEDEC_READ = 0x9f,
+RDSR = 0x5,
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
@@ -348,6 +351,25 @@ static void test_write_page_mem(void)
flash_reset();
}
+static void test_read_status_reg(void)
+{
+uint8_t r;
+
+qmp("{ 'execute': 'qom-set', 'arguments': "
+ "{'path': '/machine/soc/fmc/ssi.0/child[0]', 'property': 'WEL',
'value': true}}");
+
+
+spi_conf(CONF_ENABLE_W0);
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+
+g_assert_cmphex(r & SR_WEL, ==, SR_WEL);
+
+flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -373,6 +395,7 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/write_page", test_write_page);
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
+qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
ret = g_test_run();
--
2.30.2
[PATCH 0/1] hw: allow write_enable latch get/set
Hey everyone, My patch adds the get/set property for the write_enable latch. Currently, it is not an exposed property but this change adds QOM visibility and makes this property modifiable. Acceptance tests have also been added to verify the value in the status register (after the value is set). Thanks, Iris Iris Chen (1): hw: allow write_enable latch get/set hw/block/m25p80.c | 30 ++ tests/qtest/aspeed_smc-test.c | 20 2 files changed, 50 insertions(+) -- 2.30.2
[PATCH 1/1] hw: allow write_enable latch get/set
---
hw/block/m25p80.c | 30 ++
tests/qtest/aspeed_smc-test.c | 20
2 files changed, 50 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 430d1298a8..fb72704e5a 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -35,6 +35,7 @@
#include "qapi/error.h"
#include "trace.h"
#include "qom/object.h"
+#include "qapi/visitor.h"
/* Fields for FlashPartInfo->flags */
@@ -1646,6 +1647,31 @@ static const VMStateDescription vmstate_m25p80 = {
}
};
+static void m25p80_get_wel(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Flash *s = M25P80(obj);
+
+assert(strcmp(name, "WEL") == 0);
+
+visit_type_bool(v, name, >write_enable, errp);
+}
+
+static void m25p80_set_wel(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Flash *s = M25P80(obj);
+bool value;
+
+assert(strcmp(name, "WEL") == 0);
+
+if (!visit_type_bool(v, name, , errp)) {
+return;
+}
+
+s->write_enable = value;
+}
+
static void m25p80_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1660,6 +1686,10 @@ static void m25p80_class_init(ObjectClass *klass, void
*data)
device_class_set_props(dc, m25p80_properties);
dc->reset = m25p80_reset;
mc->pi = data;
+
+object_class_property_add(klass, "WEL", "bool",
+m25p80_get_wel,
+m25p80_set_wel, NULL, NULL);
}
static const TypeInfo m25p80_info = {
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 87b40a0ef1..af885a9c9d 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -49,6 +49,7 @@
*/
enum {
JEDEC_READ = 0x9f,
+RDSR = 0x5,
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
@@ -348,6 +349,24 @@ static void test_write_page_mem(void)
flash_reset();
}
+static void test_read_status_reg(void)
+{
+uint8_t r;
+
+ qmp("{ 'execute': 'qom-set', 'arguments': "
+ "{'path': '/machine/soc/fmc/ssi.0/child[0]', 'property': 'WEL',
'value': true}}");
+
+spi_conf(CONF_ENABLE_W0);
+ spi_ctrl_start_user();
+ writeb(ASPEED_FLASH_BASE, RDSR);
+ r = readb(ASPEED_FLASH_BASE);
+ spi_ctrl_stop_user();
+
+ g_assert_cmphex(r, ==, 0x2);
+
+ flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -373,6 +392,7 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/write_page", test_write_page);
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
+qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
ret = g_test_run();
--
2.30.2
[PATCH 1/1] hw: allow write_enable latch get/set
---
hw/block/m25p80.c | 30 ++
tests/qtest/aspeed_smc-test.c | 20
2 files changed, 50 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 430d1298a8..fb72704e5a 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -35,6 +35,7 @@
#include "qapi/error.h"
#include "trace.h"
#include "qom/object.h"
+#include "qapi/visitor.h"
/* Fields for FlashPartInfo->flags */
@@ -1646,6 +1647,31 @@ static const VMStateDescription vmstate_m25p80 = {
}
};
+static void m25p80_get_wel(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Flash *s = M25P80(obj);
+
+assert(strcmp(name, "WEL") == 0);
+
+visit_type_bool(v, name, >write_enable, errp);
+}
+
+static void m25p80_set_wel(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Flash *s = M25P80(obj);
+bool value;
+
+assert(strcmp(name, "WEL") == 0);
+
+if (!visit_type_bool(v, name, , errp)) {
+return;
+}
+
+s->write_enable = value;
+}
+
static void m25p80_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1660,6 +1686,10 @@ static void m25p80_class_init(ObjectClass *klass, void
*data)
device_class_set_props(dc, m25p80_properties);
dc->reset = m25p80_reset;
mc->pi = data;
+
+object_class_property_add(klass, "WEL", "bool",
+m25p80_get_wel,
+m25p80_set_wel, NULL, NULL);
}
static const TypeInfo m25p80_info = {
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 87b40a0ef1..af885a9c9d 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -49,6 +49,7 @@
*/
enum {
JEDEC_READ = 0x9f,
+RDSR = 0x5,
BULK_ERASE = 0xc7,
READ = 0x03,
PP = 0x02,
@@ -348,6 +349,24 @@ static void test_write_page_mem(void)
flash_reset();
}
+static void test_read_status_reg(void)
+{
+uint8_t r;
+
+ qmp("{ 'execute': 'qom-set', 'arguments': "
+ "{'path': '/machine/soc/fmc/ssi.0/child[0]', 'property': 'WEL',
'value': true}}");
+
+spi_conf(CONF_ENABLE_W0);
+ spi_ctrl_start_user();
+ writeb(ASPEED_FLASH_BASE, RDSR);
+ r = readb(ASPEED_FLASH_BASE);
+ spi_ctrl_stop_user();
+
+ g_assert_cmphex(r, ==, 0x2);
+
+ flash_reset();
+}
+
static char tmp_path[] = "/tmp/qtest.m25p80.XX";
int main(int argc, char **argv)
@@ -373,6 +392,7 @@ int main(int argc, char **argv)
qtest_add_func("/ast2400/smc/write_page", test_write_page);
qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
+qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
ret = g_test_run();
--
2.30.2
[PATCH 0/1] hw: allow write_enable latch get/set
Hey everyone, My patch adds the get/set property for the write_enable latch. Currently, it is not an exposed property but this change adds QOM visibility and makes this property modifiable. Acceptance tests have also been added to verify the value in the status register (after the value is set). Thanks, Iris Iris Chen (1): hw: allow write_enable latch get/set hw/block/m25p80.c | 30 ++ tests/qtest/aspeed_smc-test.c | 20 2 files changed, 50 insertions(+) -- 2.30.2
