Re: [Qemu-devel] [PATCH v4 3/9] target-avr: adding a sample AVR board

[Richard Henderson]

On 06/06/2016 03:37 AM, Michael Rolnik wrote:
> +void write_Rx(CPUAVRState *env, int inst, uint8_t data)
> +{
> +    env->r[inst] = data;
> +}
> +uint8_t     read_Rx(CPUAVRState *env, int inst)

Spacing.  But more importantly...

> +static
> +void sample_io_write(void *opaque, hwaddr offset, uint64_t value, unsigned 
> size)
> +{
> +    SAMPLEIOState *s = SAMPLEIO(opaque);
> +    AVRCPU *cpu = s->cpu;
> +    CPUAVRState *env = &cpu->env;
> +
> +    assert(size == 1);
> +
> +    if (AVR_IO_CPU_REGS_BASE <= offset
> +        && offset < (AVR_IO_CPU_REGS_BASE + AVR_IO_CPU_REGS_SIZE)) {
> +        return  write_Rx(env, offset - AVR_IO_CPU_REGS_BASE, value);

...

> +static
> +int         sample_io_init(DeviceState *dev)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    SAMPLEIOState *s = SAMPLEIO(dev);
> +
> +    assert(AVR_IO_SIZE <= TARGET_PAGE_SIZE);
> +
> +    s->cpu = AVR_CPU(qemu_get_cpu(0));
> +
> +    memory_region_init_io(
> +            &s->iomem,
> +            OBJECT(s),
> +            &sample_io_ops,
> +            s,
> +            TYPE_SAMPLEIO,
> +            AVR_IO_SIZE);
> +    sysbus_init_mmio(sbd, &s->iomem);

This is a memory-mapped device, and you can't modify cpu registers with a 
memory write.

You can read cpu registers, since all TCG globals will have been synced back to 
ENV before the memory operation.  But they'll still be live in TB, and so the 
next instruction may not see the value that you just wrote.

If this write is really something that you need to support, then you'll have to 
figure out some other way of implementing it.

My first guess at a workable solution is to keep the page for this memory 
device read-only.  Detect writes via tlb_fill and throw a special exception. 
In do_interrupt, either handle the write (probably tricky); or set a flag in 
ENV, which in turn sets a flag in TB, which in turn tells translate to use a 
function (helper_writeb?) instead of using tcg_gen_qemu_st_tl.  That helper 
would be marked such that modifications to TCG registers is expected.  Then 
clear the flag in ENV so that subsequent stores use the normal path.


r~