[Qemu-devel] Should we check arguments for function `from_bcd` and `to_bcd`?

[Qemu-devel] Should we check arguments for function `from_bcd` and `to_bcd`?

[Su Hang]

When I was reading 'qemu/hw/timer/m48t59.c'(Line:328) and run with
`make check-qtest-ppc`,
I found when write an invalid value 0xc to address 0x1FFF,
`from_bcd` return 12 instead of raising an exception(or error).

"""(qemu/hw/timer/m48t59.c)
    case 0x1FFF:
    case 0x07FF:
        /* year */
    tmp = from_bcd(val);
    if (tmp >= 0 && tmp <= 99) {
"""


"""(qemu/include/qemu/bcd.h)
/* Convert a byte between binary and BCD.  */
static inline uint8_t to_bcd(uint8_t val)
{
    return ((val / 10) << 4) | (val % 10);
}

static inline uint8_t from_bcd(uint8_t val)
{
    return ((val >> 4) * 10) + (val & 0x0f);
}
"""

Su Hang

Re: [Qemu-devel] Should we check arguments for function `from_bcd` and `to_bcd`?

[Paolo Bonzini]

On 22/03/2018 08:03, Su Hang wrote:
> When I was reading 'qemu/hw/timer/m48t59.c'(Line:328) and run with
> `make check-qtest-ppc`,
> I found when write an invalid value 0xc to address 0x1FFF,
> `from_bcd` return 12 instead of raising an exception(or error).

Each device probably has a different behavior when a wrong value is
written to a register that expects valid BCD.  Therefore, if you want to
model that, you have to fix it in hw/timer/m48t59.c, not in from_bcd and
to_bcd.

However, note that anything that the guest does should never cause an
assertion.

Thanks,

Paolo

> """(qemu/hw/timer/m48t59.c)
>     case 0x1FFF:
>     case 0x07FF:
>         /* year */
>     tmp = from_bcd(val);
>     if (tmp >= 0 && tmp <= 99) {
> """
> 
> 
> """(qemu/include/qemu/bcd.h)
> /* Convert a byte between binary and BCD.  */
> static inline uint8_t to_bcd(uint8_t val)
> {
>     return ((val / 10) << 4) | (val % 10);
> }
> 
> static inline uint8_t from_bcd(uint8_t val)
> {
>     return ((val >> 4) * 10) + (val & 0x0f);
> }
> """
> 
> Su Hang
> 

Re: [Qemu-devel] Should we check arguments for function `from_bcd` and `to_bcd`?

[Su Hang]

Thanks for your reply. :-)

Su Hang

"Paolo Bonzini" <pbonzini@redhat.com>wrote:
> On 22/03/2018 08:03, Su Hang wrote:
> > When I was reading 'qemu/hw/timer/m48t59.c'(Line:328) and run with
> > `make check-qtest-ppc`,
> > I found when write an invalid value 0xc to address 0x1FFF,
> > `from_bcd` return 12 instead of raising an exception(or error).
> 
> Each device probably has a different behavior when a wrong value is
> written to a register that expects valid BCD.  Therefore, if you want to
> model that, you have to fix it in hw/timer/m48t59.c, not in from_bcd and
> to_bcd.
> 
> However, note that anything that the guest does should never cause an
> assertion.
> 
> Thanks,
> 
> Paolo
> 
> > """(qemu/hw/timer/m48t59.c)
> >     case 0x1FFF:
> >     case 0x07FF:
> >         /* year */
> >     tmp = from_bcd(val);
> >     if (tmp >= 0 && tmp <= 99) {
> > """
> > 
> > 
> > """(qemu/include/qemu/bcd.h)
> > /* Convert a byte between binary and BCD.  */
> > static inline uint8_t to_bcd(uint8_t val)
> > {
> >     return ((val / 10) << 4) | (val % 10);
> > }
> > 
> > static inline uint8_t from_bcd(uint8_t val)
> > {
> >     return ((val >> 4) * 10) + (val & 0x0f);
> > }
> > """
> > 
> > Su Hang
> > 
>