|
|
|
|
|
|
|
|
|
|
xen-devel
Re: [Xen-devel] [PATCH] 2/3: MCA/MCE correctable error handling
>+} __attribute__((packed));
I think it was a general agreement that it is not a good idea (non-portable to
non-GNU compilers) to put things like this in public headers. I think by
properly
ordering things you can get away without this (and almost without padding).
>+struct mcinfo_global {
>...
>+ uint16_t mc_socketid;
>+ uint16_t mc_coreid;
>+ uint16_t mc_vcpu_id;
Unless mc_vcpu_id is intended for that purpose, this neglects hyperthreading (I
know, AMD doesn't use this, but the interface should be vendor neutral).
If mc_vcpu_id is meant for that purpose, its naming is ambiguous.
If mc_vcpu_id is meant as a vcpuid, then ordering things os that mc_domid and
mc_vcpu_id are contiguous would seem more natural (making eventual
examination in raw memory less confusing).
>+/* sizeof(struct mcinfo_global) + 6 * sizeof(struct mcinfo_bank) == 200.
>+ * This is enough space to store mc information of up to six banks.
>+ */
>+#define MCINFO_MAXSIZE (204 - sizeof(uint32_t))
Why don't you use the sizeof()-s from the description? If this is really needed
for some reason, then having 200 in the description and 204 in the macro is
at least confusing...
> /* Frame containing list of mfns containing list of mfns containing p2m.
> */
> xen_pfn_t pfn_to_mfn_frame_list_list;
> unsigned long nmi_reason;
>+ struct arch_mc_info mc_info; /* machine check information */
> uint64_t pad[32];
> };
Are you sure it is appropriate to add a member here without reducing the
padding member?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
|
|