Re: unwind(8): refactor & simplify refcounting
On Tue, Nov 12, 2019 at 05:45:38PM +0100, Florian Obser wrote: > Did I get this right? I'd appreciate it if someone could give this a > once over. > > Since resolve() switched to a callback mechanism all uw_resolver objects > pass through resolve() and either asr_resolve_done() or > ub_resolve_done(). > With that we can pull resolver_ref() and resolver_unref() into those > functions to make the reference counting easier. > Only check_resolver is special since it needs to refcount the to be > checked resolver. But the resolver doing the actual work is > automatically refcounted by resolve() and *_resolve_done(). > One last piece of the puzzle is to track the uw_resolver object in > cb_data so that the *_resolve_done() functions have access to it. > This also allowes us to remove the ad-hoc passing of the resolver in > query_imsg. Since the callback functions all need access to the > resolver that did the work we pass it in as first argument. > Reviewed the code, did some tests and it all looks good. One nit: I would have declared a typedef for the callback funtion type to be used in the struct resolver_cb_data and the prototype and the definition of resolve(), it makes those lines easier to read. But ok anyway, -Otto > diff --git resolver.c resolver.c > index d1dce2dec71..2b7d81d29fc 100644 > --- resolver.c > +++ resolver.c > @@ -92,14 +92,11 @@ struct uw_resolver { > int64_t histogram[nitems(histogram_limits)]; > }; > > -struct check_resolver_data { > - struct uw_resolver *res; > - struct uw_resolver *check_res; > -}; > - > struct resolver_cb_data { > - void(*cb)(void *, int, void *, int, int, char *); > - void*data; > + void(*cb)(struct uw_resolver *, void *, int, void *, > + int, int, char *); > + void*data; > + struct uw_resolver *res; > }; > > __dead void resolver_shutdown(void); > @@ -108,9 +105,10 @@ void > resolver_dispatch_frontend(int, short, void *); > void resolver_dispatch_captiveportal(int, short, void *); > void resolver_dispatch_main(int, short, void *); > int resolve(struct uw_resolver *, const char*, int, int, > - void*, void (*cb)(void *, int, void *, int, int, > - char *)); > -void resolve_done(void *, int, void *, int, int, char *); > + void*, void (*cb)(struct uw_resolver *, void *, > + int, void *, int, int, char *)); > +void resolve_done(struct uw_resolver *, void *, int, void *, > + int, int, char *); > void ub_resolve_done(void *, int, void *, int, int, char *, >int); > void asr_resolve_done(struct asr_result *, void *); > @@ -129,8 +127,8 @@ void set_forwarders_oppdot(struct > uw_resolver *, > void resolver_check_timo(int, short, void *); > void resolver_free_timo(int, short, void *); > void check_resolver(struct uw_resolver *); > -void check_resolver_done(void *, int, void *, int, int, > - char *); > +void check_resolver_done(struct uw_resolver *, void *, int, > + void *, int, int, char *); > void schedule_recheck_all_resolvers(void); > int check_forwarders_changed(struct uw_forwarder_head *, >struct uw_forwarder_head *); > @@ -154,8 +152,8 @@ int > check_captive_portal_changed(struct uw_conf *, >struct uw_conf *); > void trust_anchor_resolve(void); > void trust_anchor_timo(int, short, void *); > -void trust_anchor_resolve_done(void *, int, void *, int, > - int, char *); > +void trust_anchor_resolve_done(struct uw_resolver *, void *, > + int, void *, int, int, char *); > void add_autoconf_forwarders(struct imsg_rdns_proposal *); > void rem_autoconf_forwarders(struct imsg_rdns_proposal *); > struct uw_forwarder *find_forwarder(struct uw_forwarder_head *, > @@ -480,14 +478,10 @@ resolver_dispatch_frontend(int fd, short event, void > *bula) > log_debug("%s: choosing %s", __func__, > uw_resolver_type_str[res->type]); > > - query_imsg->resolver = res; > - resolver_ref(res); > - > clock_gettime(CLOCK_MONOTONIC, _imsg->tp); > > - if ((resolve(res, query_imsg->qname, query_imsg->t, > -
Re: Remove NetChip from cdce
On Wed, Nov 13, 2019 at 06:40:00PM -0700, Aaron Bieber wrote: > Hi, > > I have a raspberry pi 0 that attaches as: > cdce0 at uhub0 port 3 configuration 2 interface 0 "Linux 4.19.75+ with \ > 2098.usb RNDIS/Ethernet Gadget" rev 2.00/4.19 addr 10 > > Unfortunately the cdce interface does not work with this particular device. > > On the linux end, it turns out that cdc and rndis are being presented. When > NetChip is removed from the cdce driver, urndis(4) picks up the slack and > things work as expected! Linux also has the product id defined as RNDIS. The thing is that Linux provides multiple "configurations" (as in: urndis(4), cdce(4), ...) over the same device. Hardcoding this combination of vendor/product id will make cdce(4) always attach, even though the configuration is actually urndis(4). In this case Linux improved well enough so that our match function checking for class/subclass should be the right thing to do. Thus, ok patrick@ > Debugged with patrick@ who is awesome and I now owe a beer or two for his > help! :D > > OK? > > Cheers, > Aaron > > diff --git a/sys/dev/usb/if_cdce.c b/sys/dev/usb/if_cdce.c > index abf0521ccd6..816bdc29c10 100644 > --- a/sys/dev/usb/if_cdce.c > +++ b/sys/dev/usb/if_cdce.c > @@ -100,7 +100,6 @@ const struct cdce_type cdce_devs[] = { > {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_CRC32 }, > {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_CRC32 }, > {{ USB_VENDOR_GMATE, USB_PRODUCT_GMATE_YP3X00 }, 0 }, > -{{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 }, > {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 }, > {{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION }, > }; > > -- > PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A 4AF0 1F81 112D 62A9 ADCE >
Remove NetChip from cdce
Hi, I have a raspberry pi 0 that attaches as: cdce0 at uhub0 port 3 configuration 2 interface 0 "Linux 4.19.75+ with \ 2098.usb RNDIS/Ethernet Gadget" rev 2.00/4.19 addr 10 Unfortunately the cdce interface does not work with this particular device. On the linux end, it turns out that cdc and rndis are being presented. When NetChip is removed from the cdce driver, urndis(4) picks up the slack and things work as expected! Linux also has the product id defined as RNDIS. Debugged with patrick@ who is awesome and I now owe a beer or two for his help! :D OK? Cheers, Aaron diff --git a/sys/dev/usb/if_cdce.c b/sys/dev/usb/if_cdce.c index abf0521ccd6..816bdc29c10 100644 --- a/sys/dev/usb/if_cdce.c +++ b/sys/dev/usb/if_cdce.c @@ -100,7 +100,6 @@ const struct cdce_type cdce_devs[] = { {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_CRC32 }, {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_CRC32 }, {{ USB_VENDOR_GMATE, USB_PRODUCT_GMATE_YP3X00 }, 0 }, -{{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 }, {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 }, {{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION }, }; -- PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A 4AF0 1F81 112D 62A9 ADCE
Re: ix(4) need support for X553
Apart from the obvious style(9) problems, can anyone give me guidance on how to approach this diff? I'm quite uneasy at the amount of changes but these nics are quite important, they're all over the place on Atom C3xxx boards which are pretty much the main "next step up" from APUs. I have an A2SDi-2C-HLN4F that I need to put in service soon (not so bothered about the ix on this box, I'm using 4xSFP+ ixl, but giving it a spin) - applying my merged diff (https://junkpile.org/ixgbe.diff2) and adding PCI_PRODUCT_INTEL_X550EM_A_1G_T_L things work so far in a quick test. - MACs look sane - no problems seen in rx/tx at 1G link speed, I haven't tried other speeds; looking at Linux diffs it seems it may need a little change for 10M - haven't tried jumbos yet - seems fairly reliable so far but tx performance is a bit poor, ~500M max sends from tcpbench - rx speeds are fine - NFS works - dmesg/pcidevs below in case they're of interest On 2019/10/30 21:43, Stuart Henderson wrote: > On 2019/10/30 07:34, sven falempin wrote: > > https://github.com/dohnuts/wip/blob/master/ixgbe.diff > > > > Needs lots of cleaning > > > > On Wed, Oct 30, 2019 at 6:59 AM Joerg Goltermann wrote: > > > > > Hello, > > > > > > I have a new Lanner NCA-1510A (with a Intel C3558) which has some > > > X553 SGMII ethernet ports. Unfortunately there is no support in ix(4) > > > for this type. > > > > > > Is anyone working on an update for the ix(4) to support the new > > > hardware? > > > > > > I took a look at the actual ix(4) and it's a bit confusing. I'm > > > not sure about the "version" of this driver compared to FreeBSD/ > > > Intel version. > > > > > > Wouldn't it be nice to have a more "stock" version of the driver. This > > > would make it much easier to port updates from FreeBSD/Intel to OpenBSD. > > > > > > Any feedback is appreciated. > > > > > > - Joerg > > > > > Oh I've got a machine coming soon and I have a nasty feeling it's going to > have one of those .. > > That diff is stale, I've merged conflicts at https://junkpile.org/ixgbe.diff2, > and it builds but totally untested. > $ sysctl hw hw.machine=amd64 hw.model=Intel(R) Atom(TM) CPU C3338 @ 1.50GHz hw.ncpu=2 hw.byteorder=1234 hw.pagesize=4096 hw.disknames=sd0:f232b1bd76ed931b hw.diskcount=1 hw.sensors.cpu0.temp0=32.00 degC hw.cpuspeed=2200 hw.setperf=199 hw.vendor=Supermicro hw.product=Super Server hw.version=0123456789 hw.serialno=0123456789 hw.uuid=----3cecef004fd4 hw.physmem=8530952192 hw.usermem=8530939904 hw.ncpufound=2 hw.allowpowerdown=1 hw.perfpolicy=manual hw.smt=0 hw.ncpuonline=2 OpenBSD 6.6-current (GENERIC.MP) #1: Wed Nov 13 23:40:03 UTC 2019 sthen@xxx:/sys/arch/amd64/compile/GENERIC.MP real mem = 8530952192 (8135MB) avail mem = 8260022272 (7877MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f0c3000 (32 entries) bios0: vendor American Megatrends Inc. version "1.1c" date 06/25/2019 bios0: Supermicro Super Server acpi0 at bios0: ACPI 6.1 acpi0: sleep states S0 S4 S5 acpi0: tables DSDT FACP FPDT FIDT SPMI MCFG WDAT APIC BDAT HPET UEFI SSDT DMAR HEST BERT ERST EINJ WSMT acpi0: wakeup devices XHC1(S4) OBL1(S4) LAN1(S4) PEX0(S4) LAN2(S4) LAN3(S4) PEX1(S4) PEX6(S4) PEX7(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimcfg0 at acpi0 acpimcfg0: addr 0xe000, bus 0-255 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 12 (boot processor) cpu0: Intel(R) Atom(TM) CPU C3338 @ 1.50GHz, 2200.42 MHz, 06-5f-01 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SMEP,ERMS,MPX,RDSEED,SMAP,CLFLUSHOPT,PT,SHA,MD_CLEAR,IBRS,IBPB,STIBP,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu0: 2MB 64b/line 16-way L2 cache cpu0: cannot disable silicon debug cpu0: smt 0, core 6, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 25MHz cpu0: mwait min=64, max=64, C-substates=0.2.0.2, IBE cpu1 at mainbus0: apid 24 (application processor) cpu1: Intel(R) Atom(TM) CPU C3338 @ 1.50GHz, 2200.02 MHz, 06-5f-01 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SMEP,ERMS,MPX,RDSEED,SMAP,CLFLUSHOPT,PT,SHA,MD_CLEAR,IBRS,IBPB,STIBP,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu1: 2MB 64b/line 16-way L2 cache cpu1: cannot disable silicon debug cpu1: smt 0, core 12, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpihpet0 at acpi0: 2399 Hz acpiprt0 at acpi0: bus 0
Re: sysupgrade: Allow to use another directory for the sets
On 12/11/2019 19:02, Renaud Allard wrote: On 12/11/2019 08:29, Theo de Raadt wrote: Renaud, please test it for me like this: sysupgrade -d / This interface is dangerously incorrect. What about this one? ping. I haven't seen any reply on the prefix based patch, but what about also making -k (Keep the files in /home/_sysupgrade) implicit in case -d is used? smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH: 1/3] MMIO handler in vmm(4)
On Sat, Nov 02, 2019 at 06:40:52AM +0900, Iori YONEJI wrote: > On Tue, Oct 29, 2019 at 02:17:28AM -0700, Mike Larkin wrote: > > On Thu, Oct 24, 2019 at 08:54:58AM +0900, Iori YONEJI wrote: > > > Hello tech@, > > > > > > Here is the patch discussed in the previous email. This part mainly > > > covers changes in the declaration part and fault handlers. > > > > > > > Hello, > > > > I read through the three diffs and have some feedback. > > > > First, please reformat all 3 diffs using style(9) guidelines. There are many > > spaces vs tabs issues in the diffs. > Thank you for your review. I'll fix the issues on this weekend. > > First of all, the reason of the spaces vs. tabs issues was due to > mangling by the mailer. I fixed this now, but I'm not aware of other > types of style issues, even I will go through the patches to find other > style mismatches after started working on them. > > > Second, there seems to be quite a bit of important code missing here: > Most of them convinced me, but let me leave a few comment for them. > Iori Yoneji and I are discussing this in another thread, but for the benefit of searchability/posterity, see below for a summary. > 1. Segment base and limit check > > 1. There appears to be no handling of segment base and limits for 32 bit > >instructions. These need to be read from the gdt and strictly adhered > >to. For 64 bit mode, it's not as important, unless the guest has enabled > >LMSLE (long mode segment limit enable) on AMD CPUs, in which case the > >limits need to be checked also. If I recall correctly, there are also > >some different rules that need to be followed for 32 bit segment use > >relating to permission checks and segment types. > I wasn't aware of limit check, but I think vcpu would be triggered #GP > if the segment was out of range instead of EPT violation, wouldn't it? > The SDM seems to be a bit vague here about the rules for checking segment limits in VMX non-root mode (eg, inside a VM) and whether that causes an exit or not based on various configurations, specifically around segment limits. This is likely an issue only for 32 bit guests (we can block LMSLE). > 2. Privilege level check > > 2. For that matter, there appears to be no handling of any permission or > >privilege checks in the instruction emulator. This means any privilege > >level can read or write any memory in the VM. > Sorry, it is my very big fault. I must check CPU mode in next post. > In the generic EPT case (not a segment limit issue), this is likely handled properly already. We are discussing if it makes sense to put the permission checks in anyway, in case this code ends up being used in other places. > 3-4. 'A'ccessed and 'D'irty bit management > > 3. There appears to be no handling of the updating of the 'A'ccessed or > >'D'irty bits on successful page table walk and writing to a page > >computed by that translation. Granted, this probably needs to go into > >the existing translate_gva function for the 'A' bit, but the 'D' bit > >needs to also be handled here. > > > > 4. There appears to be no handling of the updating of the 'A' bit in the > >GDT for the segment descriptor in use. > Yes, but I'm not sure what would be the expected use of A/D bit > management on MMIO region because it wouldn't be subject to swap in/out. > I think I will understand the expected behavior after working on it, > however, I couldn't get how it compromise the virtual machine features. > There are certain degenerate cases that might require this, and we are unclear under these circumstances if the CPU does this for us or if the emulator needs to. As pointed out above, the need for A/D bits in this particular case is likely minimal, but in the same theme as above, we may want to make the code work generically instead of assuming the emulator will only ever be used for accesses in the MMIO region. -ml > 5-7. Prefixes > > 5. The code seems to ignore any segment prefix override bytes. > > > > 6. The code seems to ignore forbidden instruction prefixes (like rex.w > >or lock prefixes being placed before instructions where they don't > >make sense or are prohibited by the SDM) > > > > 7. For that matter, operand size encoding prefixes don't seem to be > >honoured at all > Yes, there are some prefixes I missed in the patches. I will enhance > them to handle those prefixes. > One thing to confirm: Is it OK not to calculate in the segment registers > (whether override-ed or not) but just count the bytes, because we know > the address the instruction intended to access? > > 8. %rflags > > 8. %rflags is not being updated in any scenario. It looks like there were > >a few #defines for things like VMM_EMUL_ADD/SUB, which would require > >setting various flags in %rflags, but I can't seem to find the code that > >actually does these instructions, so maybe these are just old #defines? > MOV is the only instruction in my