On 4/16/20 6:03 AM, Peter Maydell wrote:
>> +#ifdef CONFIG_USER_ONLY
>> + memset(&info->attrs, 0, sizeof(info->attrs));
>
> Could just write "info->attrs = {};" ?
Not quite. Correct syntax would be attrs = (MemTxAttrs){ }.
I don't see that as an improvement over memset though.
>> + int16_t mem_off_first[2];
>> + int16_t reg_off_first[2];
>> + int16_t reg_off_last[2];
>
> It would be helpful to document what these actually are,
> and in particular what the behaviour is if the whole thing
> fits in a single page. (Judging by the code, the elements
> at index 1 for the 2nd page are set to -1 ?)
Yes, that's right. I've added some more detail here.
>> + intptr_t reg_off_first = -1, reg_off_last = -1, reg_off_split;
>> + intptr_t mem_off_last, mem_off_split;
>> + intptr_t page_split, elt_split;
>> + intptr_t i;
>
> intptr_t seems like a funny type to be using here, since these
> aren't actually related to pointers as far as I can tell.
They're used as array indexes. If we use "int", the compiler keeps
re-extending the value.
>> + memset(info, -1, offsetof(SVEContLdSt, page));
>
> I guess this isn't conceptually much different from zeroing
> out integer struct fields, but it feels a bit less safe somehow.
It seems easier than setting 9 fields separately...
>> + page_split = -(addr | TARGET_PAGE_MASK);
>
> What is the negation for ?
Computation of remaining bytes in the page:
-(x | TARGET_PAGE_MASK)
= -(x | -TARGET_PAGE_SIZE)
= TARGET_PAGE_SIZE - (x & -TARGET_PAGE_SIZE)
We use this all over qemu.
r~
