Re: [Qemu-devel] [PATCH 6/6] qga: RFC: guest-side retrieval of fw_cfg file

[Laszlo Ersek <lersek@redhat.com>]

On 03/19/15 19:27, Kevin O'Connor wrote:
> On Tue, Mar 17, 2015 at 01:38:06PM +0100, Laszlo Ersek wrote:
>> On 03/16/15 15:15, Gabriel L. Somlo wrote:
>>>   1. I can't for the life of me figure out how to stop gcc -O2
>>>      from assuming the if() test below is ALWAYS FALSE, and thus
>>>      optimizing it out completely. For now I've forced -O0 on
>>>      the entire function, but for some reason fw_cfg_read(&fcfile, ...)
>>>      does not appear to count as potentially modifying fcfile...
> [...]
>>> +static void
>>> +fw_cfg_read(void *buf, int len)
>>> +{
>>> +    insb(PORT_FW_CFG_DATA, buf, len);
>>> +}
> [...]
>> I think fw_cfg_read() is inlined under -O2, and the insb() from that
>> function is somehow confusing gcc.
>>
>> From "/usr/include/sys/io.h", on my RHEL-7.1 laptop:
>>
>> static __inline void
>> insb (unsigned short int __port, void *__addr, unsigned long int __count)
>> {
>>   __asm__ __volatile__ ("cld ; rep ; insb":"=D" (__addr), "=c" (__count)
>>                         :"d" (__port), "0" (__addr), "1" (__count));
>> }
> 
> My read of this is that gcc knows it must emit the instruction, and it
> knows that __addr and __count can change.  But, it doesn't know that
> the memory at *__addr can change.  I'd see if a barrier() fixes it.
> 
> See the section on "clobber" at:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
> 
> In particular:
> 
>   You can use a trick to avoid this if the size of the memory being
>   accessed is known at compile time. For example, if accessing ten bytes
>   of a string, use a memory input like:
> 
>   {"m"( ({ struct { char x[10]; } *p = (void *)ptr ; *p; }) )}.

I think you nailed it, thanks.

Laszlo