[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 03/14] xen/x86: Make mfn_to_gfn typesafe
Hi George,
On 28/05/2019 18:29, George Dunlap wrote:
On 5/20/19 4:13 PM, Julien Grall wrote:
Hi,
On 10/05/2019 14:25, Julien Grall wrote:
On 10/05/2019 14:24, Jan Beulich wrote:
On 10.05.19 at 15:02, <julien.grall@xxxxxxx> wrote:
On 10/05/2019 12:35, Jan Beulich wrote:
On 07.05.19 at 17:14, <julien.grall@xxxxxxx> wrote:
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -474,7 +474,8 @@ static inline void trace_resync(int event,
mfn_t gmfn)
if ( tb_init_done )
{
/* Convert gmfn to gfn */
- unsigned long gfn = mfn_to_gfn(current->domain, gmfn);
+ unsigned long gfn = gfn_x(mfn_to_gfn(current->domain,
gmfn));
+
__trace_var(event, 0/*!tsc*/, sizeof(gfn), &gfn);
}
Can't you use gfn_t here, and hence avoid the gfn_x()? Same again
further
down.
Because __trace_var will export the value to the guest. I wasn't sure
whether we
can safely consider that gfn_t is exactly the same as unsigned long in
debug-build.
Hmm, well - see the definition of gfn_t. George, what do you think?
I know what's the current definition. My point is we never made that
assumption before. In all honesty, sure assumption would definitely
help in a few places, but I think we ought to safeguard with
BUILD_BUG(...).
George, do you have any opinions?
Sorry, not sure how I missed this question earlier.
The __trace_var() call here is designed to be generic: whatever type or
size gfn is, it will copy the whole thing. Tracing is explicitly not
meant to be a stable interface -- the toolstack needs to track the
hypervisor in terms of what it's going to kick out.
So, having gfn be a gfn_t in this case should be fine; in fact it should
be _better_ than unsigned long, since if gfn_t ever does change size,
the trace record will change size appropriately. If that happens,
xenalyze will need to be modified to understand how to deal with the new
size, but that's expected.
All that to say: it looks like Jan's suggestion of having gfn_t here
would be better.
Make sense, thank you for the answer. I will respin the series.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
