Jordan Niethe's on March 24, 2020 9:45 am:
> On Mon, Mar 23, 2020 at 6:37 PM Nicholas Piggin <npig...@gmail.com> wrote:
>>
>> Jordan Niethe's on March 20, 2020 3:18 pm:
>> I'm a bit against using partially constructed opaque type for things
>> like this, even if it is in the code that knows about the type. We
>> could modify ppc_inst_prefixed() to assert that pad is equal to zero
>> (or some poisoned value) if it's not prefixed. Or do some validation
>> on the suffix if it is.
> Okay what about something like:
> +static inline ppc_inst ppc_inst_read(const void *ptr)
> +{
> + u32 prefix, suffix;
> + prefix = *(u32 *)ptr;
> + if (prefix >> 26 == 1)
> + suffix = *((u32 *)ptr + 1);
> + else
> + suffix = 0;
> + return PPC_INST_PREFIX(prefix, suffix);
> +}
Sure, if that's the best way to test prefix.
>> Although there's proably no real performance or atomicity issues here,
>> I'd be pleased if we could do a case for prefixed and a case for non
>> prefixed, and store the non-prefixed with "std". Just for the principle
>> of not having half-written instructions in the image.
> Do you mean store the prefixed with std?
Oops, yes.
>> > @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
>> > if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
>> > bp->address = a;
>> > bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
>> > - patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
>> > return bp;
>> > }
>> > }
>>
>> Why is this okay to remove?
> When we only had word instructions the bpt was just patched in here
> once and that was that.
> With prefixed instructions bp->instr + 1 might be the suffix. So I
> moved putting the breakpoint to insert_bpts():
> patch_instruction(bp->instr + ppc_inst_len(instr), PPC_INST(bpinstr));
Ah okay.
Thanks,
Nick
