A glimpse at PL011 emulation in safe Rust

Paolo Bonzini Wed, 04 Dec 2024 12:59:36 -0800

Hi all,

this is finally a look at what the PL011 device model would look like
in Rust, without having to write unsafe so often.  The attached file is
a synopsis of the C code, the Rust code and my commentary (the C code is
partly rearranged to match the ordering of the Rust code).


The code compiles on my computer, but it needs some functionality that
is still untested or stubbed out.  Getting to here will take some time
and some reviews.  But in the meanwhile, this is already useful to show
what having Rust available in QEMU *actually* looks like for people who
have to write a device.

While preparing this I noticed some logic changes between the C and Rust
version, for which I will send patches in due course.  Otherwise, the
device logic was changed as little as possible and especially the naming
style.  So, while the coding style is mostly fixed to what rustfmt
produces, some adjustments to the names and how they are referred to
can be made (absolutely tell me if you prefer "registers::Data" or
"regs::Data" or "DataReg"!).

Please review and be as brutal as needed. :)  For your convenience I
copied the commentary after my signature, with line numbers; quoting it
is probably the easiest way to reply, though of course in some cases
you might prefer to quote code instead.

If we want to continue, please remember that people will need to step
in sooner rather than later (e.g. adding bindings to QAPI and DMA; but
also doing reviews to ensure that the code is not impenetrable to
anyone but me).  Or if we want to abort because there is not enough
benefit, this is also the time to decide it.

Paolo


10 These are the APIs that PL011 needs.  Note that "bindings" is not
11 imported, all that is necessary from C code has been wrapped.

14 "prelude" provides commonly used structs and traits from all over the place.

18 "qom_isa" is a macro that helps with defining QOM superclasses.

44 Rust only allows indexing arrays with usize indices.  Adding
45 the Index implementation here removes some casts elsewhere.

52 Equivalent code is generated automatically by the Rust compiler
53 (with "#[derive(Display)]" on the RegisterOffsets enum).

70 I am not very happy with the handling of bitmasks.  It is certainly
71 possible to do better.  The ".0" is because the code tries to use a
72 type wrapping the bitfields, but not hard enough (irq_level, irq_enable,
73 IRQMASK are all u32).

75 Rust supports operator overloading, but not yet in "const" context (which
76 includes "const" declarations of course, but also "static" declarations).
77 This is the main missing bit that I would like from the language.
78 Until it's in, we'll probably have to be creative.

85 Needed for migration to look at the struct.

88 These are effectively bitfields, but portable.  They are handled by an
89 external crate.  The ugly-ish part about it is that it doesn't support
90 const operation very well, so it is hard to define constants for recurring
91 values of the registers (e.g. DR_BE in the C code).

95 read_fifo could be a [registers::Data; PL011_FIFO_DEPTH] too.  Not sure
96 how that would work for vmstate, though.

98 Note how the mutable part of the device is in a separate struct.  This is
99 because all accesses check that the BQL is taken, and also that it is
100 not dropped in the middle of the access.

116 This [BqlRefCell] is what does the BQL checks.

118 Owned<> means that instance_init will add a reference to the Clock and
119 instance_finalize will drop it.

122 All superclasses have to be listed, at least for now.

124 I took a slightly different approach to the device_id, compared to the
125 C code, mostly to test whether the bindings to QOM could do subclasses
126 of classes that written themselves in Rust.

130 These "unsafe impl" are the only occurrence of unsafe in the whole device.
131 The ObjectType trait is marked unsafe because it tells the QOM C code
132 things about the layout of the PL011State struct that the user is promising.
133 Often, unsafe traits are generated by macros that can promise safety.

138 As will be visible later, implementing class_init for "standard" classes
139 is mostly automatic, but you do have to recurse up the class hierarchy.

142 This trait defines what goes in TypeInfo.

145 So for example this ends up in the TypeInfo's .instance_post_init field.

148 This "Builder" thing is a common idiom in Rust whenever not all fields
149 are always initialized.  Note that the MemoryRegionOps callbacks are
150 methods implemented by struct PL011State.

155 Rust code will have a stricter separation between instance_init and
156 instance_post_init.  The former only takes care of initializing the struct
157 (the compiler checks that all fields are initialized!), the latter does
158 other initialization such as sysbus_init_irq and sysbus_init_mmio.

172 This trait defines what goes in DeviceClass.

173 This can become a "const" with a newer version of Rust.

177 This one too.

180 Realize and reset cannot be functions in the DeviceImpl trait, because
181 the code also needs to express their *absence* (i.e. using the inherited
182 implementation).  It may be possible in the future to use procedural
183 macros to make things look a bit more natural.  But this is at the cost
184 of hidden magic, so it is a debatable benefit and for now I am leaving
185 everything visible.

189 Here I am taking a slightly different approach to callbacks;
190 they are all included in a trait instead of being methods
191 implemented by PL011State.  The reason for this is only to
192 figure out the advantages and disadvantages; this one seems
193 more "natural" but if one has more than one character device
194 backend things will get more complicated.  I am leaning
195 towards using methods for character device ops as well, which
196 is also more similar to the C API.  I started with this other
197 idea but only because I hadn't yet figured out methods

202 Due to the separation between registers and the rest, I
203 removed external interrupt update from put_fifo (and made
204 it #[must_use] to not forget).

221 The bulk of read and write is implemented on PL011Registers.  read must
222 also return whether to invoke qemu_chr_fe_accept_input() because it can
223 cause reentrancy in PL011Registers (which is an abort!).  "ControlFlow"
224 is how the code currently represents it, but it's probably easiest to
225 return a (bool, u32) pair.  Patches are welcome.

227 Here are bitfields in action.

240 Another case in which bitmasks need more work.

243 The RSR layout is the same as bits 8-15 of DR.  This is visible in
244 the types.  Including the "error clear" part in the name is probably
245 wrong because the register is only used for the reads part.  It's
246 not a "write 1 clears" registers, for example.  This brings another
247 question: what is the best way to note open improvements in the files?

297 Again some differences due to read and write being implemented on a
298 separate struct:
299
300 - read/write gets the register decoded into an enum
301
302 - write send back whether to look for interrupts
303
304 - write also needs to know the character backend.  Alternatively
305 PL011Registers could know its outer PL011State, or
306 "char_backend.write_all(&ch);" could be sent down as a
307 closure

381 This #[must_use] asks the caller to check the result (to call
382 update() or to pass it up).

428 The difference between bitfields here and ints below is a bit jarring.
429 I guess if one doesn't like it, they could use u32 like in C for both
430 registers and interrupts; nobody would complain to be honest.  Using the
431 language's features makes it clear where they leave something to be
432 desired, at least (and I don't feel like undoing the work that Manos
433 did in converting the registers to bitfields...).

463 This is still the implementation of PL011Registers.

476 And this as well.  This is *not* the reset method on DeviceState.

491 This line resets all FIFO flags, both transmit and receive.

523 Another #[must_use] here.

576 This is the callback for self.clock

582 This was the missing part of pl011_init.  BTW, vmstate is
583 the main missing part.

590 And here are also the remaining parts of read and write.
591
592 After borrow_mut(), "regs" provides a way to write to self.regs,
593 but only while the local variable regs is live.
594
595 Converting from hwaddr to enum returns Err(offset) if the offset is
596 not part of a known register.  This is one case where macros are good
597 at hiding code that "does the obvious thing".  Rust macros are more
598 powerful and generally do not have the same issues as C, but they can
599 make the code unreadable if done wrong.

602 Logging and tracing are the main missing pieces.

606 regs is dropped here.  Someone else can now write to self.regs.
607
608 Again, don't read too much in Break/Continue.  The intention of
609 the ControlFlow enum doesn't really match how QEMU uses it here.
610
611 accept_input is called after coming back; the character device
612 callbacks do their own borrow_mut().

638 PL011State needs a couple more forwarding methods to call into 
PL011Registers.
639 In fact, *this* is the implementation of the "reset" method in DeviceState.

667 The wrappers for qdev_prop_set_chr and sysbus_connect_irq take an
668 Owned<...> smart pointer.  This smart pointer automatically handles
669 object_ref() and object_unref().  The reason why they take Owned<...> and
670 not a normal reference is to signify that the object is heap allocated.
671 Since C gave a raw pointer, create the Owned by hand in unsafe blocks.
672 Also tell Rust not to drop it, i.e. not to call object_unref().
673
674 ... I lied, the functions take *a reference* to the pointer (that is, an
675 &Owned<...>).  This means that they *request* you to own a reference but
676 they don't steal it from you.  The functions take care of adding their own
677 reference: qemu_chr_fe_init() does it; or, if I'm not wrong, it should).
678
679 PL011State::new() gave us an Owned<PL011State>, converting it with into_raw
680 gives ownership of the reference to the C caller.

694 instance_init is mandatory in Rust; the case where you have no additional
695 fields to initialize (as is the case here) is rare enough.  This is why
696 it is an "fn" and not a "const" like REALIZE and RESET.  If needed, it
697 would be possible to write a separate wrapper for "this class only
698 differs from its superclass in its class_init method".

701 Again, the implementation is different so this is a class_init method.

715 The defaults are good. No need to override realize and reset, so the 
default of
716 None for the REALIZE and RESET constants are ok.

718 type_register_static is handled automagically by #[derive(Object)],
719 as an initial tasting of what macros could do.  The main things that
720 we could do with macros is implementing qdev properties in a type-safe
721 manner, and perhaps improve the bitmask situation.

#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/char/pl011.h"
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "hw/qdev-clock.h"
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
#include "migration/vmstate.h"
#include "chardev/char-fe.h"
#include "chardev/char-serial.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "trace.h"


... omitting register definitions ...









/* Integer Baud Rate Divider, UARTIBRD */
#define IBRD_MASK 0xffff

/* Fractional Baud Rate Divider, UARTFBRD */
#define FBRD_MASK 0x3f

/* Depth of UART FIFO in bytes, when FIFO mode is enabled (else depth == 1) */
#define PL011_FIFO_DEPTH 16

static const unsigned char pl011_id_arm[8] =
  { 0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1 };
static const unsigned char pl011_id_luminary[8] =
  { 0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1 };












static const char *pl011_regname(hwaddr offset)
{
    static const char *const rname[] = {
        [0] = "DR", [1] = "RSR", [6] = "FR", [8] = "ILPR", [9] = "IBRD",
        [10] = "FBRD", [11] = "LCRH", [12] = "CR", [13] = "IFLS", [14] = "IMSC",
        [15] = "RIS", [16] = "MIS", [17] = "ICR", [18] = "DMACR",
    };
    unsigned idx = offset >> 2;

    if (idx < ARRAY_SIZE(rname) && rname[idx]) {
        return rname[idx];
    }
    if (idx >= 0x3f8 && idx <= 0x400) {
        return "ID";
    }
    return "UNKN";
}

/* Which bits in the interrupt status matter for each outbound IRQ line ? */
static const uint32_t irqmask[] = {
    INT_E | INT_MS | INT_RT | INT_TX | INT_RX, /* combined IRQ */
    INT_RX,
    INT_TX,
    INT_RT,
    INT_MS,
    INT_E,
};






OBJECT_DECLARE_SIMPLE_TYPE(PL011State, PL011)

struct PL011State {
    SysBusDevice parent_obj;

    MemoryRegion iomem;
    uint32_t flags;
    uint32_t lcr;
    uint32_t rsr;
    uint32_t cr;
    uint32_t dmacr;
    uint32_t int_enabled;
    uint32_t int_level;
    uint32_t read_fifo[PL011_FIFO_DEPTH];
    uint32_t ilpr;
    uint32_t ibrd;
    uint32_t fbrd;
    uint32_t ifl;
    int read_pos;
    int read_count;
    int read_trigger;
    CharBackend chr;
    qemu_irq irq[6];
    Clock *clk;
    bool migrate_clk;
    const unsigned char *id;
};


















static const TypeInfo pl011_arm_info = {
    .name          = TYPE_PL011,
    .parent        = TYPE_SYS_BUS_DEVICE,
    .instance_size = sizeof(PL011State),
    .instance_init = pl011_init,
    .class_init    = pl011_class_init,
};










static const MemoryRegionOps pl011_ops = {
    .read = pl011_read,
    .write = pl011_write,
    .endianness = DEVICE_NATIVE_ENDIAN,
    .impl.min_access_size = 4,
    .impl.max_access_size = 4,
};

static void pl011_init(Object *obj)
{
    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
    PL011State *s = PL011(obj);
    int i;

    memory_region_init_io(&s->iomem, OBJECT(s), &pl011_ops, s, "pl011", 0x1000);
    sysbus_init_mmio(sbd, &s->iomem);
    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
        sysbus_init_irq(sbd, &s->irq[i]);
    }

    s->clk = qdev_init_clock_in(DEVICE(obj), "clk", pl011_clock_update, s,
                                ClockUpdate);
    s->id = pl011_id_arm;
}

static void pl011_class_init(ObjectClass *oc, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(oc);

    dc->realize = pl011_realize;
    device_class_set_legacy_reset(dc, pl011_reset);
    dc->vmsd = &vmstate_pl011;
    device_class_set_props(dc, pl011_properties);
}







static int pl011_can_receive(void *opaque)
{
    PL011State *s = (PL011State *)opaque;
    int r;

    r = s->read_count < pl011_get_fifo_depth(s);
    trace_pl011_can_receive(s->lcr, s->read_count, r);
    return r;
}

static void pl011_event(void *opaque, QEMUChrEvent event)
{
    if (event == CHR_EVENT_BREAK && !pl011_loopback_enabled(opaque)) {
        pl011_put_fifo(opaque, DR_BE);
    }
}



static void pl011_receive(void *opaque, const uint8_t *buf, int size)
{
    /*
     * In loopback mode, the RX input signal is internally disconnected
     * from the entire receiving logics; thus, all inputs are ignored,
     * and BREAK detection on RX input signal is also not performed.
     */
    if (pl011_loopback_enabled(opaque)) {
        return;
    }

    pl011_put_fifo(opaque, *buf);
}

static uint64_t pl011_read(void *opaque, hwaddr offset,
                           unsigned size)
{
    PL011State *s = (PL011State *)opaque;
    uint32_t c;
    uint64_t r;

    switch (offset >> 2) {
    case 0: /* UARTDR */
        s->flags &= ~PL011_FLAG_RXFF;
        c = s->read_fifo[s->read_pos];
        if (s->read_count > 0) {
            s->read_count--;
            s->read_pos = (s->read_pos + 1) & (pl011_get_fifo_depth(s) - 1);
        }
        if (s->read_count == 0) {
            s->flags |= PL011_FLAG_RXFE;
        }
        if (s->read_count == s->read_trigger - 1)
            s->int_level &= ~ INT_RX;
        trace_pl011_read_fifo(s->read_count);
        s->rsr = c >> 8;
        pl011_update(s);
        qemu_chr_fe_accept_input(&s->chr);
        r = c;
        break;
    case 1: /* UARTRSR */
        r = s->rsr;
        break;
    case 6: /* UARTFR */
        r = s->flags;
        break;
    case 8: /* UARTILPR */
        r = s->ilpr;
        break;
    case 9: /* UARTIBRD */
        r = s->ibrd;
        break;
    case 10: /* UARTFBRD */
        r = s->fbrd;
        break;
    case 11: /* UARTLCR_H */
        r = s->lcr;
        break;
    case 12: /* UARTCR */
        r = s->cr;
        break;
    case 13: /* UARTIFLS */
        r = s->ifl;
        break;
    case 14: /* UARTIMSC */
        r = s->int_enabled;
        break;
    case 15: /* UARTRIS */
        r = s->int_level;
        break;
    case 16: /* UARTMIS */
        r = s->int_level & s->int_enabled;
        break;
    case 18: /* UARTDMACR */
        r = s->dmacr;
        break;
    case 0x3f8 ... 0x400:
        r = s->id[(offset - 0xfe0) >> 2];
        break;
    default:
        qemu_log_mask(LOG_GUEST_ERROR,
                      "pl011_read: Bad offset 0x%x\n", (int)offset);
        r = 0;
        break;
    }

    trace_pl011_read(offset, r, pl011_regname(offset));
    return r;
}

static void pl011_write(void *opaque, hwaddr offset,
                        uint64_t value, unsigned size)
{
    PL011State *s = (PL011State *)opaque;
    unsigned char ch;

    trace_pl011_write(offset, value, pl011_regname(offset));

    switch (offset >> 2) {
    case 0: /* UARTDR */
        /* ??? Check if transmitter is enabled.  */
        ch = value;
        /* XXX this blocks entire thread. Rewrite to use
         * qemu_chr_fe_write and background I/O callbacks */
        qemu_chr_fe_write_all(&s->chr, &ch, 1);
        pl011_loopback_tx(s, ch);
        s->int_level |= INT_TX;
        pl011_update(s);
        break;
    case 1: /* UARTRSR/UARTECR */
        s->rsr = 0;
        break;
    case 6: /* UARTFR */
        /* Writes to Flag register are ignored.  */
        break;
    case 8: /* UARTILPR */
        s->ilpr = value;
        break;
    case 9: /* UARTIBRD */
        s->ibrd = value & IBRD_MASK;
        pl011_trace_baudrate_change(s);
        break;
    case 10: /* UARTFBRD */
        s->fbrd = value & FBRD_MASK;
        pl011_trace_baudrate_change(s);
        break;
    case 11: /* UARTLCR_H */
        /* Reset the FIFO state on FIFO enable or disable */
        if ((s->lcr ^ value) & LCR_FEN) {
            pl011_reset_fifo(s);
        }
        if ((s->lcr ^ value) & LCR_BRK) {
            int break_enable = value & LCR_BRK;
            qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
                              &break_enable);
            pl011_loopback_break(s, break_enable);
        }
        s->lcr = value;
        pl011_set_read_trigger(s);
        break;
    case 12: /* UARTCR */
        /* ??? Need to implement the enable bit.  */
        s->cr = value;
        pl011_loopback_mdmctrl(s);
        break;
    case 13: /* UARTIFS */
        s->ifl = value;
        pl011_set_read_trigger(s);
        break;
    case 14: /* UARTIMSC */
        s->int_enabled = value;
        pl011_update(s);
        break;
    case 17: /* UARTICR */
        s->int_level &= ~value;
        pl011_update(s);
        break;
    case 18: /* UARTDMACR */
        s->dmacr = value;
        if (value & 3) {
            qemu_log_mask(LOG_UNIMP, "pl011: DMA not implemented\n");
        }
        break;
    default:
        qemu_log_mask(LOG_GUEST_ERROR,
                      "pl011_write: Bad offset 0x%x\n", (int)offset);
    }
}






static void pl011_loopback_tx(PL011State *s, uint32_t value)
{
    if (!pl011_loopback_enabled(s)) {
        return;
    }

    /*
     * Caveat:
     *
     * In real hardware, TX loopback happens at the serial-bit level
     * and then reassembled by the RX logics back into bytes and placed
     * into the RX fifo. That is, loopback happens after TX fifo.
     *
     * Because the real hardware TX fifo is time-drained at the frame
     * rate governed by the configured serial format, some loopback
     * bytes in TX fifo may still be able to get into the RX fifo
     * that could be full at times while being drained at software
     * pace.
     *
     * In such scenario, the RX draining pace is the major factor
     * deciding which loopback bytes get into the RX fifo, unless
     * hardware flow-control is enabled.
     *
     * For simplicity, the above described is not emulated.
     */
    pl011_put_fifo(s, value);
}

static void pl011_loopback_mdmctrl(PL011State *s)
{
    uint32_t cr, fr, il;

    if (!pl011_loopback_enabled(s)) {
        return;
    }

    /*
     * Loopback software-driven modem control outputs to modem status inputs:
     *   FR.RI  <= CR.Out2
     *   FR.DCD <= CR.Out1
     *   FR.CTS <= CR.RTS
     *   FR.DSR <= CR.DTR
     *
     * The loopback happens immediately even if this call is triggered
     * by setting only CR.LBE.
     *
     * CTS/RTS updates due to enabled hardware flow controls are not
     * dealt with here.
     */
    cr = s->cr;
    fr = s->flags & ~(PL011_FLAG_RI | PL011_FLAG_DCD |
                      PL011_FLAG_DSR | PL011_FLAG_CTS);
    fr |= (cr & CR_OUT2) ? PL011_FLAG_RI  : 0;
    fr |= (cr & CR_OUT1) ? PL011_FLAG_DCD : 0;
    fr |= (cr & CR_RTS)  ? PL011_FLAG_CTS : 0;
    fr |= (cr & CR_DTR)  ? PL011_FLAG_DSR : 0;

    /* Change interrupts based on updated FR */
    il = s->int_level & ~(INT_DSR | INT_DCD | INT_CTS | INT_RI);
    il |= (fr & PL011_FLAG_DSR) ? INT_DSR : 0;
    il |= (fr & PL011_FLAG_DCD) ? INT_DCD : 0;
    il |= (fr & PL011_FLAG_CTS) ? INT_CTS : 0;
    il |= (fr & PL011_FLAG_RI)  ? INT_RI  : 0;

    s->flags = fr;
    s->int_level = il;
    pl011_update(s);
}






static void pl011_loopback_break(PL011State *s, int brk_enable)
{
    if (brk_enable) {
        pl011_loopback_tx(s, DR_BE);
    }
}

static void pl011_set_read_trigger(PL011State *s)
{
#if 0
    /* The docs say the RX interrupt is triggered when the FIFO exceeds
       the threshold.  However linux only reads the FIFO in response to an
       interrupt.  Triggering the interrupt when the FIFO is non-empty seems
       to make things work.  */
    if (s->lcr & LCR_FEN)
        s->read_trigger = (s->ifl >> 1) & 0x1c;
    else
#endif
        s->read_trigger = 1;
}

static void pl011_reset(DeviceState *dev)
{
    PL011State *s = PL011(dev);

    s->lcr = 0;
    s->rsr = 0;
    s->dmacr = 0;
    s->int_enabled = 0;
    s->int_level = 0;
    s->ilpr = 0;
    s->ibrd = 0;
    s->fbrd = 0;
    s->read_trigger = 1;
    s->ifl = 0x12;
    s->cr = 0x300;
    s->flags = 0;
    pl011_reset_fifo(s);
}

static inline void pl011_reset_fifo(PL011State *s)
{
    s->read_count = 0;
    s->read_pos = 0;

    /* Reset FIFO flags */
    s->flags &= ~(PL011_FLAG_RXFF | PL011_FLAG_TXFF);
    s->flags |= PL011_FLAG_RXFE | PL011_FLAG_TXFE;
}

static bool pl011_is_fifo_enabled(PL011State *s)
{
    return (s->lcr & LCR_FEN) != 0;
}

static bool pl011_loopback_enabled(PL011State *s)
{
    return !!(s->cr & CR_LBE);
}

static inline unsigned pl011_get_fifo_depth(PL011State *s)
{
    /* Note: FIFO depth is expected to be power-of-2 */
    return pl011_is_fifo_enabled(s) ? PL011_FIFO_DEPTH : 1;
}




static void pl011_put_fifo(void *opaque, uint32_t value)
{
    PL011State *s = (PL011State *)opaque;
    int slot;
    unsigned pipe_depth;

    pipe_depth = pl011_get_fifo_depth(s);
    slot = (s->read_pos + s->read_count) & (pipe_depth - 1);
    s->read_fifo[slot] = value;
    s->read_count++;
    s->flags &= ~PL011_FLAG_RXFE;
    trace_pl011_put_fifo(value, s->read_count);
    if (s->read_count == pipe_depth) {
        trace_pl011_put_fifo_full();
        s->flags |= PL011_FLAG_RXFF;
    }
    if (s->read_count == s->read_trigger) {
        s->int_level |= INT_RX;
        pl011_update(s);
    }
}

static int pl011_post_load(void *opaque, int version_id)
{
    PL011State* s = opaque;

    /* Sanity-check input state */
    if (s->read_pos >= ARRAY_SIZE(s->read_fifo) ||
        s->read_count > ARRAY_SIZE(s->read_fifo)) {
        return -1;
    }

    if (!pl011_is_fifo_enabled(s) && s->read_count > 0 && s->read_pos > 0) {
        /*
         * Older versions of PL011 didn't ensure that the single
         * character in the FIFO in FIFO-disabled mode is in
         * element 0 of the array; convert to follow the current
         * code's assumptions.
         */
        s->read_fifo[0] = s->read_fifo[s->read_pos];
        s->read_pos = 0;
    }

    s->ibrd &= IBRD_MASK;
    s->fbrd &= FBRD_MASK;

    return 0;
}

 ... omitting tracepoint support ... 

static void pl011_clock_update(void *opaque, ClockEvent event)
{
    PL011State *s = PL011(opaque);

    pl011_trace_baudrate_change(s);
}

 ... omitting vmstate definitions (Rust API not final) ... 















































static void pl011_realize(DeviceState *dev, Error **errp)
{
    PL011State *s = PL011(dev);

    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
                             pl011_event, NULL, s, NULL, true);
}











static void pl011_update(PL011State *s)
{
    uint32_t flags;
    int i;

    flags = s->int_level & s->int_enabled;
    trace_pl011_irq_state(flags != 0);
    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
        qemu_set_irq(s->irq[i], (flags & irqmask[i]) != 0);
    }
}




DeviceState *pl011_create(hwaddr addr, qemu_irq irq, Chardev *chr)
{
    DeviceState *dev;
    SysBusDevice *s;






    dev = qdev_new("pl011");
    s = SYS_BUS_DEVICE(dev);
    qdev_prop_set_chr(dev, "chardev", chr);
    sysbus_realize_and_unref(s, &error_fatal);
    sysbus_mmio_map(s, 0, addr);
    sysbus_connect_irq(s, 0, irq);

    return dev;
}



















static void pl011_luminary_init(Object *obj)
{
    PL011State *s = PL011(obj);

    s->id = pl011_id_luminary;
}

static const TypeInfo pl011_luminary_info = {
    .name          = TYPE_PL011_LUMINARY,
    .parent        = TYPE_PL011,
    .instance_init = pl011_luminary_init,
};





static void pl011_register_types(void)
{
    type_register_static(&pl011_arm_info);
    type_register_static(&pl011_luminary_info);
}

type_init(pl011_register_types)

use std::{
    ffi::CStr,
    mem::ManuallyDrop,
    ops::ControlFlow,
};

use pinned_init::{pin_data, pin_init, PinInit};

use qemu_api::{
    memory::{MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder, hwaddr},
    cell::BqlRefCell,
    chardev::{CharBackend, CharBackendOps, Chardev, Event},
    irq::{InterruptSource, IRQState},
    prelude::*,
    qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property},
    qom::{ClassInitImpl, ObjectImpl, ObjectType, Owned},
    sysbus::{SysBusDevice, SysBusDeviceClass},
    qom_isa,
};

use crate::{
    device_class,
    registers::{self, Interrupt},
    RegisterOffset,
};

/// Integer Baud Rate Divider, `UARTIBRD`
const IBRD_MASK: u32 = 0xffff;

/// Fractional Baud Rate Divider, `UARTFBRD`
const FBRD_MASK: u32 = 0x3f;

// Depth of UART FIFO in bytes, when FIFO mode is enabled (else depth == 1)
const PL011_FIFO_DEPTH: usize = 16;

#[derive(Clone, Copy)]
struct DeviceId(&'static [u8; 8]);

impl DeviceId {
    const ARM: Self = Self(&[0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1]);
    const LUMINARY: Self = Self(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
}

impl std::ops::Index<hwaddr> for DeviceId {
    type Output = u8;

    fn index(&self, idx: hwaddr) -> &Self::Output {
        &self.0[idx as usize]
    }
}



















/// Which bits in the interrupt status matter for each outbound IRQ line ?
const IRQMASK: [u32; 6] = [
    /* combined IRQ */
    Interrupt::E.0
        | Interrupt::MS.0
        | Interrupt::RT.0
        | Interrupt::TX.0
        | Interrupt::RX.0,
    Interrupt::RX.0,
    Interrupt::TX.0,
    Interrupt::RT.0,
    Interrupt::MS.0,
    Interrupt::E.0,
];

#[repr(C)]
#[derive(Debug, Default, qemu_api_macros::offsets)]
struct PL011Registers {
    pub flags: registers::Flags,
    pub line_control: registers::LineControl,
    pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
    pub control: registers::Control,
    pub dmacr: u32,
    pub int_enabled: u32,
    pub int_level: u32,
    pub read_fifo: [u32; PL011_FIFO_DEPTH],
    pub ilpr: u32,
    pub ibrd: u32,
    pub fbrd: u32,
    pub ifl: u32,
    pub read_pos: usize,
    pub read_count: usize,
    pub read_trigger: usize,
}

#[repr(C)]
#[derive(qemu_api_macros::Object, qemu_api_macros::offsets)]
#[pin_data]
/// PL011 Device Model in QEMU
pub struct PL011State {
    #[pin]
    pub parent_obj: SysBusDevice,
    #[pin]
    pub iomem: MemoryRegion,
    #[pin]
    pub char_backend: CharBackend,
    pub regs: BqlRefCell<PL011Registers>,
    pub interrupts: [InterruptSource; IRQMASK.len()],
    pub clock: Owned<Clock>,
    pub migrate_clock: bool,
}

qom_isa!(PL011State : SysBusDevice, DeviceState, Object);

pub struct PL011Class {
    parent_class: <SysBusDevice as ObjectType>::Class,
    /// The byte string that identifies the device.
    device_id: DeviceId,
}

unsafe impl ObjectType for PL011State {
    type Class = PL011Class;
    const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
}

impl ClassInitImpl<PL011Class> for PL011State {
    fn class_init(klass: &mut PL011Class) {
        klass.device_id = DeviceId::ARM;
        <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
    }
}

impl ObjectImpl for PL011State {
    type ParentType = SysBusDevice;

    const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init);

    fn instance_init(parent_init: impl PinInit<SysBusDevice>) -> impl PinInit<Self> {
         static PL011_OPS: MemoryRegionOps = MemoryRegionOpsBuilder::<PL011State>::new()
             .read(&PL011State::read)
             .write(&PL011State::write)
             .native_endian()
             .impl_sizes(4, 4)
             .build();

        pin_init!(&this in PL011State {
            parent_obj <- parent_init,
            iomem <- MemoryRegion::init_io(
                this, &PL011_OPS, "pl011", 0x1000,
            ),
            char_backend <- CharBackend::new(),
            regs: Default::default(),
            interrupts: Default::default(),
            clock <- Self::init_clock_in(this, "clk",
	        &Self::clock_update,
		ClockEvent::ClockUpdate),
            migrate_clock: false,
        })
    }
}


impl DeviceImpl for PL011State {
    fn properties() -> &'static [Property] {
        &device_class::PL011_PROPERTIES
    }

    fn vmsd() -> Option<&'static VMStateDescription> {
        Some(&device_class::VMSTATE_PL011)
    }
    const REALIZE: Option<fn(&Self)> = Some(Self::realize);
    const RESET: Option<fn(&Self)> = Some(Self::reset);
}






impl CharBackendOps<'_> for PL011State {
    fn can_receive(&self) -> u32 {
        let regs = self.regs.borrow();
        // trace_pl011_can_receive(s->lcr, s->read_count, r);
        u32::from(regs.read_count < regs.fifo_depth())
    }




    fn event(&self, event: Event) {
        let mut regs = self.regs.borrow_mut();
        if event == Event::CHR_EVENT_BREAK && !regs.loopback_enabled() {
            if regs.put_fifo(u32::from(registers::Data::BREAK)) {
                self.update();
            }
        }
    }

    fn receive(&self, buf: &[u8]) {
        let mut regs = self.regs.borrow_mut();
        if !regs.loopback_enabled() && !buf.is_empty() && regs.put_fifo(buf[0].into()) {
            self.update();
        }
    }
}






impl PL011Registers {
    pub fn read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, u32> {
        use RegisterOffset::*;




        ControlFlow::Break(match offset {
            DR => {
                self.flags.set_receive_fifo_full(false);
                let c = self.read_fifo[self.read_pos];
                if self.read_count > 0 {
                    self.read_count -= 1;
                    self.read_pos = (self.read_pos + 1) & (self.fifo_depth() - 1);
                }
                if self.read_count == 0 {
                    self.flags.set_receive_fifo_empty(true);
                }
                if self.read_count + 1 == self.read_trigger {
                    self.int_level &= !Interrupt::RX.0;
                }
                // Update error bits.
		self.receive_status_error_clear = registers::Data::from(c).errors();
                // Must call qemu_chr_fe_accept_input, so return Continue:
                return ControlFlow::Continue(c);
            }
            RSR => u8::from(self.receive_status_error_clear).into(),
            FR => u16::from(self.flags).into(),
            FBRD => self.fbrd,
            ILPR => self.ilpr,
            IBRD => self.ibrd,
            LCR_H => u16::from(self.line_control).into(),
            CR => u16::from(self.control).into(),
            FLS => self.ifl,
            IMSC => self.int_enabled,
            RIS => self.int_level,
            MIS => self.int_level & self.int_enabled,
            ICR => {
                // "The UARTICR Register is the interrupt clear register and is write-only"
                // Source: ARM DDI 0183G 3.3.13 Interrupt Clear Register, UARTICR
                0
            }
            DMACR => self.dmacr,
        })
    }































    pub fn write(
        &mut self,
        offset: RegisterOffset,
        value: u32,
        char_backend: &CharBackend,
    ) -> bool {
        // eprintln!("write offset {offset} value {value}");
        use RegisterOffset::*;
        match offset {
            DR => {
                // ??? Check if transmitter is enabled.
                let ch: [u8; 1] = [value as u8];
                // XXX this blocks entire thread. Rewrite to use
                // qemu_chr_fe_write and background I/O callbacks
                let _ = char_backend.write_all(&ch);
                // interrupts always checked
                let _ = self.loopback_tx(value);
                self.int_level |= Interrupt::TX.0;
                return true;
            }
            RSR => {
                self.receive_status_error_clear.reset();
            }
            FR => {
                // flag writes are ignored
            }
            ILPR => {
                self.ilpr = value;
            }
            IBRD => {
                self.ibrd = value;
            }
            FBRD => {
                self.fbrd = value;
            }
            LCR_H => {
                let value = value as u16;
                let new_val: registers::LineControl = value.into();
                // Reset the FIFO state on FIFO enable or disable
		if self.line_control.fifos_enabled() != new_val.fifos_enabled() {
                    self.reset_rx_fifo();
                }
                let update = (self.line_control.send_break() != new_val.send_break()) && {
                    let break_enable = new_val.send_break();
                    let _ = char_backend.send_break(break_enable);
                    self.loopback_break(break_enable)
                };
                self.line_control = new_val;
                self.set_read_trigger();
                return update;
            }
            CR => {
                // ??? Need to implement the enable bit.
                let value = value as u16;
                self.control = value.into();
                return self.loopback_mdmctrl();
            }
            FLS => {
                self.ifl = value;
                self.set_read_trigger();
            }
            IMSC => {
                self.int_enabled = value;
                return true;
            }
            RIS => {}
            MIS => {}
            ICR => {
                self.int_level &= !value;
                return true;
            }
            DMACR => {
                self.dmacr = value;
                if value & 3 > 0 {
                    // qemu_log_mask(LOG_UNIMP, "pl011: DMA not implemented\n");
                    eprintln!("pl011: DMA not implemented");
                }
            }
        }
        false
    }



    #[inline]
    #[must_use]
    fn loopback_tx(&mut self, value: u32) -> bool {
        // Caveat:
        //
        // In real hardware, TX loopback happens at the serial-bit level
        // and then reassembled by the RX logics back into bytes and placed
        // into the RX fifo. That is, loopback happens after TX fifo.
        //
        // Because the real hardware TX fifo is time-drained at the frame
        // rate governed by the configured serial format, some loopback
        // bytes in TX fifo may still be able to get into the RX fifo
        // that could be full at times while being drained at software
        // pace.
        //
        // In such scenario, the RX draining pace is the major factor
        // deciding which loopback bytes get into the RX fifo, unless
        // hardware flow-control is enabled.
        //
        // For simplicity, the above described is not emulated.
        self.loopback_enabled() && self.put_fifo(value)
    }





    #[must_use]
    fn loopback_mdmctrl(&mut self) -> bool {
        if !self.loopback_enabled() {
            return false;
        }

        /*
         * Loopback software-driven modem control outputs to modem status inputs:
         *   FR.RI  <= CR.Out2
         *   FR.DCD <= CR.Out1
         *   FR.CTS <= CR.RTS
         *   FR.DSR <= CR.DTR
         *
         * The loopback happens immediately even if this call is triggered
         * by setting only CR.LBE.
         *
         * CTS/RTS updates due to enabled hardware flow controls are not
         * dealt with here.
         */

        self.flags.set_ring_indicator(self.control.out_2());
        self.flags.set_data_carrier_detect(self.control.out_1());
        self.flags.set_clear_to_send(self.control.request_to_send());
        self.flags
            .set_data_set_ready(self.control.data_transmit_ready());

        // Change interrupts based on updated FR
        let mut il = self.int_level;

        il &= !Interrupt::MS.0;

        if self.flags.data_set_ready() {
            il |= Interrupt::DSR.0;
        }
        if self.flags.data_carrier_detect() {
            il |= Interrupt::DCD.0;
        }
        if self.flags.clear_to_send() {
            il |= Interrupt::CTS.0;
        }
        if self.flags.ring_indicator() {
            il |= Interrupt::RI.0;
        }
        self.int_level = il;
        true
    }

    fn loopback_break(&mut self, enable: bool) -> bool {
        enable && self.loopback_tx(u32::from(registers::Data::BREAK))
    }




    fn set_read_trigger(&mut self) {
        self.read_trigger = 1;
    }











    pub fn reset(&mut self) {



        self.line_control.reset();
        self.receive_status_error_clear.reset();
        self.dmacr = 0;
        self.int_enabled = 0;
        self.int_level = 0;
        self.ilpr = 0;
        self.ibrd = 0;
        self.fbrd = 0;
        self.read_trigger = 1;
        self.ifl = 0x12;
        self.control.reset();
        self.flags.reset();
        self.reset_rx_fifo();
    }

    pub fn reset_rx_fifo(&mut self) {
        self.read_count = 0;
        self.read_pos = 0;

        /* Reset receive FIFO flags */
        self.flags.set_receive_fifo_full(false);
        self.flags.set_receive_fifo_empty(true);
    }

    #[inline]
    pub fn fifo_enabled(&self) -> bool {
        self.line_control.fifos_enabled() == registers::Mode::FIFO
    }

    #[inline]
    pub fn loopback_enabled(&self) -> bool {
        self.control.enable_loopback()
    }

    #[inline]
    pub fn fifo_depth(&self) -> usize {
        // Note: FIFO depth is expected to be power-of-2
        if self.fifo_enabled() {
            return PL011_FIFO_DEPTH;
        }
        1
    }

    #[must_use]
    pub fn put_fifo(&mut self, value: u32) -> bool {




        let depth = self.fifo_depth();
        assert!(depth > 0);
        let slot = (self.read_pos + self.read_count) & (depth - 1);
        self.read_fifo[slot] = value;
        self.read_count += 1;
        self.flags.set_receive_fifo_empty(false);
        if self.read_count == depth {
            self.flags.set_receive_fifo_full(true);
        }

        if self.read_count == self.read_trigger {
            self.int_level |= Interrupt::RX.0;
            return true;
        }
        false
    }

    pub fn post_load(&mut self) -> Result<(), ()> {
        /* Sanity-check input state */
        if self.read_pos >= self.read_fifo.len() || self.read_count > self.read_fifo.len() {
            return Err(());
        }

        if !self.fifo_enabled() && self.read_count > 0 && self.read_pos > 0 {
            // Older versions of PL011 didn't ensure that the single
            // character in the FIFO in FIFO-disabled mode is in
            // element 0 of the array; convert to follow the current
            // code's assumptions.
            self.read_fifo[0] = self.read_fifo[self.read_pos];
            self.read_pos = 0;
        }

        self.ibrd &= IBRD_MASK;
        self.fbrd &= FBRD_MASK;

        Ok(())
    }
}








impl PL011State {
    fn clock_update(&self, _event: ClockEvent) {
        /* pl011_trace_baudrate_change(s); */
    }



    // part of pl011_init in C: 
    fn post_init(&self) {
        self.init_mmio(&self.iomem);
        for irq in self.interrupts.iter() {
            self.init_irq(irq);
        }
    }

    // part of pl011_read in C: 
    pub fn read(&self, offset: hwaddr, _size: u32) -> u64 {
        let mut regs = self.regs.borrow_mut();


        let result = match RegisterOffset::try_from(offset) {
            Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => {
                let device_id = self.get_class().device_id;
                return u64::from(device_id[(offset - 0xfe0) >> 2])
            }

            Err(_) => {
                // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
                return 0
            }
            Ok(field) => regs.read(field),
        };

        match result {
            ControlFlow::Break(value) => value.into(),
            ControlFlow::Continue(value) => {
                self.update();
                self.char_backend.accept_input();
                value.into()
            },
        }
    }

    // part of pl011_write in C: 
    pub fn write(&self, offset: hwaddr, value: u64, _size: u32) {
        let mut regs = self.regs.borrow_mut();
        if let Ok(field) = RegisterOffset::try_from(offset) {
            if regs.write(field, value as u32, &self.char_backend) {
                self.update();
            }
        } else {
            eprintln!("write bad offset {offset} value {value}");
        }
    }

    pub fn realize(&self) {
        self.char_backend.enable_handlers(self);
    }





    // part of pl011_reset in C: 
    pub fn reset(&self) {
        self.regs.borrow_mut().reset();
    }

    // part of pl011_post_load in C: 
    pub fn post_load(&self, _version_id: u32) -> Result<(), ()> {
        self.regs.borrow_mut().post_load()
    }

    pub fn update(&self) {
        let regs = self.regs.borrow();
        let flags = regs.int_level & regs.int_enabled;
        for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
            irq.set(flags & i != 0);
        }
    }
}


/// # Safety
///
/// We expect the FFI user of this function to pass a valid pointer for `chr`
/// and `irq`.
#[no_mangle]
pub unsafe extern "C" fn pl011_create(
    addr: u64,
    irq: *mut IRQState,
    chr: *mut Chardev,
) -> *mut DeviceState {
    // SAFETY: The callers promise that they have owned references.
    // pl011_create does not release them.
    let irq = unsafe { ManuallyDrop::new(Owned::<IRQState>::from_raw(irq)) };
    let chr = unsafe { ManuallyDrop::new(Owned::<Chardev>::from_raw(chr)) };

    let dev = PL011State::new();
    dev.prop_set_chr("chardev", &chr);
    dev.realize();
    dev.mmio_map(0, addr);
    dev.connect_irq(0, &irq);

    Owned::into_raw(dev.upcast())
}

#[repr(C)]
#[derive(qemu_api_macros::Object)]
#[pin_data]
/// PL011 Luminary device model.
pub struct PL011Luminary {
    #[pin]
    parent_obj: PL011State,
}

impl ObjectImpl for PL011Luminary {
    type ParentType = PL011State;

    fn instance_init(parent_init: impl PinInit<PL011State>) -> impl PinInit<Self> {
        pin_init!(PL011Luminary {
            parent_obj <- parent_init
        })
    }
}

impl ClassInitImpl<PL011Class> for PL011Luminary {
    fn class_init(klass: &mut PL011Class) {
        klass.device_id = DeviceId::LUMINARY;
        <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
    }
}

qom_isa!(PL011Luminary : PL011State, SysBusDevice, DeviceState, Object);

unsafe impl ObjectType for PL011Luminary {
    type Class = <PL011State as ObjectType>::Class;
    const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
}

impl DeviceImpl for PL011Luminary {}










These are the APIs that PL011 needs.  Note that "bindings" is not
imported, all that is necessary from C code has been wrapped.


"prelude" provides some commonly used structs and traits from all over the place.



"qom_isa" is a macro that helps with defining QOM superclasses.

























Rust only allows indexing arrays with usize indices.  Adding
the Index implementation here removes some casts elsewhere.






Equivalent code is generated automatically by the Rust compiler
(with "#[derive(Display)]" on the RegisterOffsets enum).
















I am not very happy with the handling of bitmasks.  It is certainly
possible to do better.  The ".0" is because the code tries to use a
type wrapping the bitfields, but not hard enough (irq_level, irq_enable,
IRQMASK are all u32).

Rust supports operator overloading, but not yet in "const" context (which
includes "const" declarations of course, but also "static" declarations).
This is the main missing bit that I would like from the language.
Until it's in, we'll probably have to be creative.






Needed for migration to look at the struct.


These are effectively bitfields, but portable.  They are handled by an
external crate.  The ugly-ish part about it is that it doesn't support
const operation very well, so it is hard to define constants for recurring
values of the registers (e.g. DR_BE in the C code).



read_fifo could be a [registers::Data; PL011_FIFO_DEPTH] too.  Not sure
how that would work for vmstate, though.

Note how the mutable part of the device is in a separate struct.  This is
because all accesses check that the BQL is taken, and also that it is
not dropped in the middle of the access.















This is what does the BQL checks.

Owned<> means that instance_init will add a reference to the Clock and
instance_finalize will drop it.


All superclasses have to be listed, at least for now.

I took a slightly different approach to the device_id, compared to the
C code, mostly to test whether the bindings to QOM could do subclasses
of classes that written themselves in Rust.



These "unsafe impl" are the only occurrence of unsafe in the whole device.
The ObjectType trait is marked unsafe because it tells the QOM C code
things about the layout of the PL011State struct that the user is promising.
Often, unsafe traits are generated by macros that can promise safety.




As will be visible later, implementing class_init for "standard" classes
is mostly automatic, but you do have to recurse up the class hierarchy.


This trait defines what goes in TypeInfo.


So for example this ends up in the TypeInfo's .instance_post_init field.


This "Builder" thing is a common idiom in Rust whenever not all fields
are always initialized.  Note that the MemoryRegionOps callbacks are
methods implemented by struct PL011State.




Rust code will have a stricter separation between instance_init and
instance_post_init.  The former only takes care of initializing the struct
(the compiler checks that all fields are initialized!), the latter does
other initialization such as sysbus_init_irq and sysbus_init_mmio.













This trait defines what goes in DeviceClass.
This can become a "const" with a newer version of Rust.



This one too.


Realize and reset cannot be functions in the DeviceImpl trait, because
the code also needs to express their *absence* (i.e. using the inherited
implementation).  It may be possible in the future to use procedural
macros to make things look a bit more natural.  But this is at the cost
of hidden magic, so it is a debatable benefit and for now I am leaving
everything visible.



Here I am taking a slightly different approach to callbacks;
they are all included in a trait instead of being methods
implemented by PL011State.  The reason for this is only to
figure out the advantages and disadvantages; this one seems
more "natural" but if one has more than one character device
backend things will get more complicated.  I am leaning
towards using methods for character device ops as well, which
is also more similar to the C API.  I started with this other
idea but only because I hadn't yet figured out methods.




Due to the separation between registers and the rest, I
removed external interrupt update from put_fifo (and made
it #[must_use] to not forget).
















The bulk of read and write is implemented on PL011Registers.  read must
also return whether to invoke qemu_chr_fe_accept_input() because it can
cause reentrancy in PL011Registers (which is an abort!).  "ControlFlow"
is how the code currently represents it, but it's probably easiest to
return a (bool, u32) pair.  Patches are welcome.

Here are bitfields in action.












Another case in which bitmasks need more work.


The RSR layout is the same as bits 8-15 of DR.  This is visible in
the types.  Including the "error clear" part in the name is probably
wrong because the register is only used for the reads part.  It's
not a "write 1 clears" registers, for example.  This brings another
question: what is the best way to note open improvements in the files?

















































Again some differences due to read and write being implemented on a
separate struct:

- read/write gets the register decoded into an enum

- write send back whether to look for interrupts

- write also needs to know the character backend.  Alternatively
  PL011Registers could know its outer PL011State, or
  "char_backend.write_all(&ch);" could be sent down as a
  closure









































































This #[must_use] asks the caller to check the result (to call
update() or to pass it up).













































The difference between bitfields here and ints below is a bit jarring.
I guess if one doesn't like it, they could use u32 like in C for both
registers and interrupts; nobody would complain to be honest.  Using the
language's features makes it clear where they leave something to be
desired, at least (and I don't feel like undoing the work that Manos
did in converting the registers to bitfields...).





























This is still the implementation of PL011Registers.












And this as well.  This is *not* the reset method on DeviceState.














This line resets all FIFO flags, both transmit and receive.































Another #[must_use] here.




















































This is the callback for self.clock





This was the missing part of pl011_init.  BTW, vmstate is
the main missing part.






And here are also the remaining parts of read and write.

After borrow_mut(), "regs" provides a way to write to self.regs,
but only while the local variable regs is live.

Converting from hwaddr to enum returns Err(offset) if the offset is
not part of a known register.  This is one case where macros are good
at hiding code that "does the obvious thing".  Rust macros are more
powerful and generally do not have the same issues as C, but they can
make the code unreadable if done wrong.


Logging and tracing are the main missing pieces.



regs is dropped here.  Someone else can now write to self.regs.

Again, don't read too much in Break/Continue.  The intention of
the ControlFlow enum doesn't really match how QEMU uses it here.

accept_input is called after coming back; the character device
callbacks do their own borrow_mut().

























PL011State needs a couple more forwarding methods to call into PL011Registers.
In fact, *this* is the implementation of the "reset" method in DeviceState.



























The wrappers for qdev_prop_set_chr and sysbus_connect_irq take an
Owned<...> smart pointer.  This smart pointer automatically handles
object_ref() and object_unref().  The reason why they take Owned<...> and
not a normal reference is to signify that the object is heap allocated.
Since C gave a raw pointer, create the Owned by hand in unsafe blocks.
Also tell Rust not to drop it, i.e. not to call object_unref().

... I lied, the functions take *a reference* to the pointer (that is, an
&Owned<...>).  This means that they *request* you to own a reference but
they don't steal it from you.  The functions take care of adding their own
reference: qemu_chr_fe_init() does it; or, if I'm not wrong, it should).

PL011State::new() gave us an Owned, converting it with into_raw
gives ownership of the reference to the C caller.













instance_init is mandatory in Rust; the case where you have no additional
fields to initialize (as is the case here) is rare enough.  This is why
it is an "fn" and not a "const" like REALIZE and RESET.  If needed, it
would be possible to write a separate wrapper for "this class only
differs from its superclass in its class_init method".


Again, the implementation is different so this is a class_init method.













The defaults are good. No need to override realize and reset, so the default of
None for the REALIZE and RESET constants are ok.

type_register_static is handled automagically by #[derive(Object)],
as an initial tasting of what macros could do.  The main things that
we could do with macros is implementing qdev properties in a type-safe
manner, and perhaps improve the bitmask situation.

A glimpse at PL011 emulation in safe Rust

Reply via email to