Re: [PATCH 3/5] ftgmac100: check RX and TX buffer alignment

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/7/20 9:36 AM, Cédric Le Goater wrote:
> On 1/7/20 9:27 AM, Philippe Mathieu-Daudé wrote:
>> On 1/7/20 8:34 AM, Cédric Le Goater wrote:
>>> These buffers should be aligned on 16 bytes.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>    hw/net/ftgmac100.c | 13 +++++++++++++
>>>    1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
>>> index 86ac25894a89..051f7b7af2d6 100644
>>> --- a/hw/net/ftgmac100.c
>>> +++ b/hw/net/ftgmac100.c
>>> @@ -198,6 +198,8 @@ typedef struct {
>>>        uint32_t        des3;
>>>    } FTGMAC100Desc;
>>>    +#define FTGMAC100_DESC_ALIGNMENT 16
>>> +
>>>    /*
>>>     * Specific RTL8211E MII Registers
>>>     */
>>> @@ -722,6 +724,12 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
>>>            s->itc = value;
>>>            break;
>>>        case FTGMAC100_RXR_BADR: /* Ring buffer address */
>>> +        if (!QEMU_IS_ALIGNED(value, FTGMAC100_DESC_ALIGNMENT)) {
>>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad RX buffer alignment 0x%"
>>> +                          HWADDR_PRIx "\n", __func__, value);
>>> +            return;
>>
>> What is the hardware behavior?
> 
> This is not documented :/

What happens if we don't return?

>>
>>> +        }
>>> +
>>>            s->rx_ring = value;
>>>            s->rx_descriptor = s->rx_ring;
>>>            break;
>>> @@ -731,6 +739,11 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
>>>            break;
>>>          case FTGMAC100_NPTXR_BADR: /* Transmit buffer address */
>>> +        if (!QEMU_IS_ALIGNED(value, FTGMAC100_DESC_ALIGNMENT)) {
>>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad TX buffer alignment 0x%"
>>> +                          HWADDR_PRIx "\n", __func__, value);
>>> +            return;
>>> +        }
>>>            s->tx_ring = value;
>>>            s->tx_descriptor = s->tx_ring;
>>>            break;
>>>
>>
>