On 08.08.2023 10:48, Oleksii wrote:
> On Mon, 2023-08-07 at 15:29 +0200, Jan Beulich wrote:
>> On 03.08.2023 14:05, Oleksii Kurochko wrote:
>>> +static uint32_t read_instr(unsigned long pc)
>>> +{
>>> + uint16_t instr16 = *(uint16_t *)pc;
>>> +
>>> + if ( GET_INSN_LENGTH(instr16) == 2 )
>>> + return (uint32_t)instr16;
>>
>> (I don't think this cast is needed.)
>>
>>> + else
>>> + return *(uint32_t *)pc;
>>> +}
>>
>> ... there still being a double read here, do you perhaps mean to
>> make a statement (that this code isn't safe to use on guest code)?
> I wonder if it'll be safe to read 16 bytes at a time then we won't have
> double read ( if you meant that first 16 bytes are read twice ):
>
> static uint32_t read_instr(unsigned long pc)
> {
> uint16_t instr16 = *(uint16_t *)pc;
>
> if ( GET_INSN_LENGTH(instr16) == 2 )
> return (uint32_t)instr16;
> else{
> // return *(uint32_t *)pc;
>
> uint16_t next_16 = *((uint16_t *)pc + 1);
> return ((uint32_t)instr16 << sizeof(instr16)) + next_16;
> }
> }
Whether this is safe for guest code depends further on what underlying
mappings there are. Surely you can't simply cast a guest add (coming
in as "unsigned long pc") to a hypervisor address. So as it stands the
function can only ever be used on Xen code anyway.
Jan
