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
|
Home