Re: [Xen-devel] [PATCH vtpm v2 01/12] Add ioread/iowrite functions to mini-os

[Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>]

On 10/08/2012 09:36 AM, Ian Campbell wrote:
> On Mon, 2012-10-08 at 12:48 +0100, Ian Campbell wrote:
>> I was about to apply nos. 1 & 3-7 but there was a build error:
>>         In file included from mini-os.c:23:
>>         ../grub-upstream/netboot/etherboot.h:526: error: conflicting types 
>> for âinet_atonâ
>>         
>> /local/scratch/ianc/devel/committer.git/stubdom/lwip-x86_64/src/include/ipv4/lwip/inet.h:44:
>>  note: previous declaration of âinet_atonâ was here
>>         mini-os.c: In function âload_moduleâ:
>>         mini-os.c:244: warning: statement with no effect
>>         mini-os.c: In function âmainâ:
>>         mini-os.c:747: warning: cast from pointer to integer of different 
>> size
>>         make[2]: *** 
>> [/local/scratch/ianc/devel/committer.git/stubdom/grub-x86_64/mini-os.o] 
>> Error 1
>>         make[2]: *** Waiting for unfinished jobs....
> This was down to #6 "minios: add select definition to sys/time.h when
> HAVE_LIBC is defined" -- which I guess caused an indirect inclusion of
> the problematic header? I'm compiling on Debian Squeeze BTW.
It looks like this is happening because
extras/mini-os/include/posix/sys/select.h does a #include <lwip/sockets.h>.

lwip.sockets.h defines struct timeval if LWIP_TIMEVAL_PRIVATE is 1,
which in extra/mini-os/include/lwipopts.h is set to 0. So the result is
we always use the struct timeval in extras/mini-os/sys/time.h and the
lwip/sockets.h include is redundant.

I think the correct solution is to remove the lwip include. Unless for
some reason people want to allow for setting LWIP_TIMEVAL_PRIVATE to 1
mini-os, but if we want to allow that case then extras/sys/time.h needs
to also be patched.

I tested removing it and the build error goes away. I'll send a patch
shortly.
>
> So I have gone ahead and committed 1, 3-5 & 7, IOW:
> minios: setup fpu and sse in mini-os
> minios: add CONFIG_XC conditional
> minios: Disable the mfn_is_ram() check, it doesn't work correctly on all
> minios: Add endian, byteswap, and wordsize macros to mini-os
> minios: Add ioread/iowrite functions to mini-os
>
>> There was also a slew of warnings -- but I think many of them might be
>> normal for a stubdom build (I'll have to check).
> These turned out to be pre-existing.
>
>> BTW I was skipping, this time round:
>>
>> #2 because Samuel seemed to have some additional thoughts this morning.
>>
>> #8 because I wanted to have another quick read through before I applied
>> it.
>>
>> #9 and #10 are already applied.
>>
>> #11 hasn't been acked, although I see George has reviewed it at least
>> somewhat. Hopefully one or the other of us (or someone else) will find
>> the time shortly.
>>
>> #12 is already applied
>>
>> Ian.
>>
>> On Fri, 2012-10-05 at 19:01 +0100, Matthew Fioravante wrote:
>>> This patch adds iowritexx() and ioreadxx() functions for interacting
>>> with hardware memory to mini-os. The functions are available in a header
>>> iorw.h
>>>
>>> Signed-off-by: Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>
>>> Acked-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxxx>
>>> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>>>
>>> diff --git a/extras/mini-os/arch/x86/iorw.c b/extras/mini-os/arch/x86/iorw.c
>>> new file mode 100644
>>> index 0000000..3080769
>>> --- /dev/null
>>> +++ b/extras/mini-os/arch/x86/iorw.c
>>> @@ -0,0 +1,35 @@
>>> +#include <mini-os/iorw.h>
>>> +
>>> +void iowrite8(volatile void* addr, uint8_t val)
>>> +{
>>> +   *((volatile uint8_t*)addr) = val;
>>> +}
>>> +void iowrite16(volatile void* addr, uint16_t val)
>>> +{
>>> +   *((volatile uint16_t*)addr) = val;
>>> +}
>>> +void iowrite32(volatile void* addr, uint32_t val)
>>> +{
>>> +   *((volatile uint32_t*)addr) = val;
>>> +}
>>> +void iowrite64(volatile void* addr, uint64_t val)
>>> +{
>>> +   *((volatile uint64_t*)addr) = val;
>>> +}
>>> +
>>> +uint8_t ioread8(volatile void* addr)
>>> +{
>>> +   return *((volatile uint8_t*) addr);
>>> +}
>>> +uint16_t ioread16(volatile void* addr)
>>> +{
>>> +   return *((volatile uint16_t*) addr);
>>> +}
>>> +uint32_t ioread32(volatile void* addr)
>>> +{
>>> +   return *((volatile uint32_t*) addr);
>>> +}
>>> +uint64_t ioread64(volatile void* addr)
>>> +{
>>> +   return *((volatile uint64_t*) addr);
>>> +}
>>> diff --git a/extras/mini-os/include/iorw.h b/extras/mini-os/include/iorw.h
>>> new file mode 100644
>>> index 0000000..d5ec065
>>> --- /dev/null
>>> +++ b/extras/mini-os/include/iorw.h
>>> @@ -0,0 +1,16 @@
>>> +#ifndef MINIOS_IORW_H
>>> +#define MINIOS_IORW_H
>>> +
>>> +#include <mini-os/types.h>
>>> +
>>> +void iowrite8(volatile void* addr, uint8_t val);
>>> +void iowrite16(volatile void* addr, uint16_t val);
>>> +void iowrite32(volatile void* addr, uint32_t val);
>>> +void iowrite64(volatile void* addr, uint64_t val);
>>> +
>>> +uint8_t ioread8(volatile void* addr);
>>> +uint16_t ioread16(volatile void* addr);
>>> +uint32_t ioread32(volatile void* addr);
>>> +uint64_t ioread64(volatile void* addr);
>>> +
>>> +#endif
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxx
>> http://lists.xen.org/xen-devel
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel