On 2016年05月10日 01:29, Peter Maydell wrote:
> +static MemTxResult gicd_writeb(GICv3State *s, hwaddr offset,
> + uint64_t value, MemTxAttrs attrs)
> +{
> + /* Most GICv3 distributor registers do not support byte accesses. */
> + switch (offset) {
> + case GICD_CPENDSGIR ... GICD_CPENDSGIR + 0xf:
> + case GICD_SPENDSGIR ... GICD_SPENDSGIR + 0xf:
> + case GICD_ITARGETSR ... GICD_ITARGETSR + 0x3ff:
> + /* This GIC implementation always has affinity routing enabled,
> + * so these registers are all RAZ/WI.
> + */
> + return MEMTX_OK;
> + case GICD_IPRIORITYR ... GICD_IPRIORITYR + 0x3ff:
> + {
> + int irq = offset - GICD_IPRIORITYR;
> +
> + gicd_write_ipriorityr(s, attrs, irq, value);
> + gicv3_update(s, irq, 1);
The GICv3 SPEC says:
"
When affinity routing is enabled for the security state of an interrupt:
• GICR_IPRIORITYR<n> is used instead of GICD_IPRIORITYR<n> where n = 0
to 7 (that
is, for SGIs and PPIs).
• GICD_IPRIORITYR<n> is RAZ/WI where n = 0 to 7.
"
So I think it shouldn't call gicv3_update if attrs.secure is true and
irq < 32. And it should check the parameter irq in gicv3_update().
> + return MEMTX_OK;
> + }
> + default:
> + return MEMTX_ERROR;
> + }
> +}
> +
> +static MemTxResult gicd_readw(GICv3State *s, hwaddr offset,
> + uint64_t *data, MemTxAttrs attrs)
> +{
> + /* Only GICD_SETSPI_NSR, GICD_CLRSPI_NSR, GICD_SETSPI_SR and
> GICD_SETSPI_NSR
> + * support 16 bit accesses, and those registers are all part of the
> + * optional message-based SPI feature which this GIC does not currently
> + * implement (ie for us GICD_TYPER.MBIS == 0), so for us they are
> + * reserved.
> + */
> + return MEMTX_ERROR;
> +}
> +
> +static MemTxResult gicd_writew(GICv3State *s, hwaddr offset,
> + uint64_t value, MemTxAttrs attrs)
> +{
> + /* Only GICD_SETSPI_NSR, GICD_CLRSPI_NSR, GICD_SETSPI_SR and
> GICD_SETSPI_NSR
> + * support 16 bit accesses, and those registers are all part of the
> + * optional message-based SPI feature which this GIC does not currently
> + * implement (ie for us GICD_TYPER.MBIS == 0), so for us they are
> + * reserved.
> + */
> + return MEMTX_ERROR;
> +}
> +
> +static MemTxResult gicd_readl(GICv3State *s, hwaddr offset,
> + uint64_t *data, MemTxAttrs attrs)
> +{
> + /* Almost all GICv3 distributor registers are 32-bit.
> + * Note that WO registers must return an UNKNOWN value on reads,
> + * not an abort.
> + */
> +
> + switch (offset) {
> + case GICD_CTLR:
> + if (!attrs.secure && !(s->gicd_ctlr & GICD_CTLR_DS)) {
> + /* The NS view of the GICD_CTLR sees only certain bits:
> + * + bit [31] (RWP) is an alias of the Secure bit [31]
> + * + bit [4] (ARE_NS) is an alias of Secure bit [5]
> + * + bit [1] (EnableGrp1A) is an alias of Secure bit [1] if
> + * NS affinity routing is enabled, otherwise RES0
> + * + bit [0] (EnableGrp1) is an alias of Secure bit [1] if
> + * NS affinity routing is not enabled, otherwise RES0
> + * Since for QEMU affinity routing is always enabled
> + * for both S and NS this means that bits [4] and [5] are
> + * both always 1, and we can simply make the NS view
> + * be bits 31, 4 and 1 of the S view.
> + */
> + *data = s->gicd_ctlr & (GICD_CTLR_ARE_NS |
As you said, we make the NS view be bit 4 of the S view. So the
GICD_CTLR_ARE_NS should be GICD_CTLR_ARE_S, right?
> + GICD_CTLR_EN_GRP1NS |
> + GICD_CTLR_RWP);
> + } else {
> + *data = s->gicd_ctlr;
> + }
> + return MEMTX_OK;
--
Shannon
