Re: [TECH TOPIC] Rust for Linux

Vegard Nossum
2021-07-19T16:02:41Z

On 7/19/21 3:15 PM, Wedson Almeida Filho wrote:
> On Mon, Jul 19, 2021 at 01:24:49PM +0100, Wedson Almeida Filho wrote:
>> On Fri, Jul 09, 2021 at 12:13:25AM +0200, Linus Walleij wrote:
>>> I have seen that QEMU has a piece of code for the Arm PrimeCell
>>> PL061 GPIO block which corresponds to drivers/gpio/gpio-pl061.c
>>> Note that this hardware apart from being used in all Arm reference
>>> designs is used on ARMv4T systems that are not supported by
>>> LLVM but only GCC, which might complicate things.
>>
>> Here is a working PL061 driver in Rust (converted form the C one):
>> https://raw.githubusercontent.com/wedsonaf/linux/pl061/drivers/gpio/gpio_pl061_rust.rs
>
> I'm also attaching an html rending of the C and Rust versions side by side where
> I try to line the definitions up to make it easier to contrast the two
> implementations.
>

This is really cool :-) As a Rust noob, I have a few questions:

1. I'm curious about some of the writeb() vs. try_writeb() calls:

fn direction_output(data: &Ref<DeviceData>, offset: u32, value: bool) ->
Result {
let woffset = bit(offset + 2).into();
let _guard = data.lock();
let pl061 = data.resources().ok_or(Error::ENXIO)?;
pl061.base.try_writeb((value as u8) << offset, woffset)?;
let mut gpiodir = pl061.base.readb(GPIODIR);
gpiodir |= bit(offset);
pl061.base.writeb(gpiodir, GPIODIR);

// gpio value is set again, because pl061 doesn't allow to set
value of a gpio pin before
// configuring it in OUT mode.
pl061.base.try_writeb((value as u8) << offset, woffset)?;
Ok(())
}

Here you have try_writeb() (and error return) where there was just a
writeb() without any error handling in the C version. Is this what
Miguel was answering a bit down the thread where the address is computed
((value as u8) << offset) so it _needs_ to use the try_() version?

If offset can be anything but a "correct" value here, should there be a
check for that somewhere else and then the computed value can be
subsequently treated as safe (i.e. there's a second try_writeb() in the
function that now presumably does the runtime check a second time,
redundantly)?

2. In many places you have the C code:

struct pl061 *pl061 = dev_get_drvdata(dev);

with the equivalent Rust code as:

let pl061 = data.resources().ok_or(Error::ENXIO)?;

Why doesn't the C code need to check for errors here? Or put
differently, why can the Rust version fail?

3. In probe() you have:

data.resources().ok_or(Error::ENXIO)?.base.writeb(0, GPIOIE); // disable
irqs
data.registrations()
.ok_or(Error::ENXIO)?
.gpio_chip
.register_with_irq(PL061_GPIO_NR, None, dev, data.clone(), irq)?;

So here, if .register_with_irq() or any of the other ?-marked calls
fail, then the function returns and the local variable "data" and all
its members are freed/deallocated using destructors like in C++ RAII?

Thanks,

Vegard