RE: [Xen-ia64-devel][PATCH] SMBios table support in XEN

["Zhang, Xing Z" <xing.z.zhang@xxxxxxxxx>]

Hi Alex:
        Thx your advice. When I modify the patch, I found
a problem that is hardly to keep away from. 
        I do a hypercall in setup_guest() to get UUID of XEN,
at this time the context belongs to DOM0. So the hypercall 
would return UUID info of dom0 ( it return current->domain->handle) which is 
zero. I can't find another 
place to put the codes because we need properly pass the value
to GFW hob. 
        Do you have any advice? I want to add a hypercall interface 
in GFW to do that.

Good good study,day day up ! ^_^
-Wing(zhang xin)

OTC,Intel Corporation
>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@xxxxxx]
>Sent: 2007年8月28日 5:42
>To: Zhang, Xing Z
>Cc: xen-ia64-devel
>Subject: RE: [Xen-ia64-devel][PATCH] SMBios table support in
>XEN
>
>Hi,
>
>   I'll point out the specific place I was thinking below, along
>with a
>couple other issues.  When I test this with the latest GFW, I'm
>also
>getting a couple of these on domain startup:
>
>xencomm_privcmd_xen_version: unknown version op 17
>
>And I'm not seeing an SMBIOS table with dmidecode in the guest.
>Is
>there still something missing?  Thanks,
>
>       Alex
>
>
>On Fri, 2007-08-24 at 13:11 +0800, Zhang, Xing Z wrote:
>> +/* Founctions for building SMBios table */
>> +static void hypercall_xen_version(int xc_handle, domid_t
>dom, int
>> type, void *val, int len)
>> +{
>
>   I think this needs to be more strict in it's "type"
>validation.
>Currently XENVER_guest_handle returns one thing, anything at
>all passed
>to it returns a version.  The version stuff seems a little
>redundant
>with xc_version().  Could we use that instead?
>
>> +    if ( type != XENVER_guest_handle )
>> +    {
>> +        DECLARE_HYPERCALL;
>> +
>> +        hypercall.op = __HYPERVISOR_xen_version;
>> +        hypercall.arg[0] = __HYPERVISOR_xen_version;
>> +
>> +        lock_pages(val, len);
>> +        do_xen_hypercall(xc_handle, &hypercall);
>> +        unlock_pages(val, len);
>> +
>> +        memcpy(val, &hypercall.arg[1], len);
>> +    }
>> +    else
>> +    {
>> +        DECLARE_DOMCTL;
>> +
>> +        domctl.cmd = XEN_DOMCTL_getdomaininfo;
>> +        domctl.domain = dom;
>> +
>> +        if (xc_domctl(xc_handle, &domctl) < 0)
>> +        {
>> +            PERROR("Get XEN UUID failed, SMBios may be built
>wrong!
>> \n");
>> +            memset(val, 0x0, len);
>> +            return;
>> +        }
>> +
>> +        memcpy(val, domctl.u.getdomaininfo.handle, len);
>> +    }
>>  }
>...
>> @@ -1068,15 +1142,31 @@ setup_guest(int xc_handle, uint32_t
>dom,
>>
>>      vcpus = domctl.u.getdomaininfo.max_vcpu_id + 1;
>>
>> -    // Hand-off state passed to guest firmware
>> -    if (xc_ia64_build_hob(xc_handle, dom, dom_memsize,
>vcpus,
>> nvram_start) < 0) {
>> +    // Get Xen version info for building SMBios table
>> +    {
>> +        uint8_t uuid[XENVER_UUID_LEN]; // This will break if
>> xen_domain_handle_t is not uint8_t[16]
>
>   This is where I thought maybe we should use a
>xen_domain_handle_t
>like Keir did in uptream:
>
>    xen_domain_handle_t uuid;
>
>    ...
>
>    BUILD_BUG_ON(sizeof(xen_domain_handle_t) != 16);
>
>Also, these should be declared at the beginning of the function
>instead
>of patched in to the middle by a sub-scope (this will also fix
>the
>indenting of the xc_ia64_build_hob() call below).
>
>> +        uint32_t xen_version;
>> +        char xen_extra_version[XEN_EXTRAVERSION_LEN];
>> +
>> +        hypercall_xen_version(xc_handle, dom,
>XENVER_guest_handle,
>> +                            (void *)uuid, sizeof(uuid));
>> +        hypercall_xen_version(xc_handle, dom,
>XENVER_version,
>> +                            (void *)&xen_version,
>> sizeof(xen_version));
>> +        hypercall_xen_version(xc_handle, dom,
>XENVER_extraversion,
>> +                            (void *)xen_extra_version,
>> sizeof(xen_extra_version));
>> +
>> +    // Hand-off state passed to guest firmware
>> +    if (xc_ia64_build_hob(xc_handle, dom, dom_memsize,
>vcpus,
>> nvram_start,
>> +                uuid, xen_version, xen_extra_version) < 0) {
>>          PERROR("Could not build hob\n");
>>          goto error_out;
>>      }
>>
>--
>Alex Williamson                             HP Open Source & Linux
>Org.

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel